[Sema] Add flag to optimize building swiftmodule files preserving type info for LLDB #34612

xymus · 2020-11-06T18:32:48Z

Add a new frontend flag -experimental-skip-non-inlinable-function-bodies-without-types to skip parsing and type-checking non-inlinable functions bodies that don't define nested types. This flag can be used as an alternative to -experimental-skip-non-inlinable-function-bodies to keep all type info used by LLDB.

rdar://problem/71130519

xymus · 2020-11-06T18:33:03Z

@swift-ci Please smoke test

nkcsgexi

Makes sense to me!

xymus · 2020-11-06T21:06:21Z

@swift-ci Please smoke test

xymus · 2020-11-06T22:32:15Z

@swift-ci Please test

swift-ci · 2020-11-06T23:31:20Z

Build failed
Swift Test Linux Platform
Git Sha - 526adc67e63655f11ec09af6a2b5a411d1ea7c3a

swift-ci · 2020-11-07T01:06:50Z

Build failed
Swift Test OS X Platform
Git Sha - 526adc67e63655f11ec09af6a2b5a411d1ea7c3a

xymus · 2020-11-09T18:02:23Z

@swift-ci Please smoke test

xymus · 2020-11-09T21:14:28Z

@swift-ci Please smoke test

bnbarham

LGTM

bnbarham · 2020-11-09T21:29:47Z

test/Frontend/skip-function-bodies.swift

-// %FileCheck %s --check-prefixes CHECK,CHECK-NONINLINE-ONLY < %t/Skip.noninlinable.sil
-// %FileCheck %s --check-prefixes CHECK,CHECK-ALL-ONLY < %t/Skip.all.sil


Ouch. Not sure how I missed that, thanks! I think I changed to outputting to a file quite late...

bnbarham · 2020-11-09T21:31:03Z

test/Frontend/skip-function-bodies.swift

@@ -58,16 +62,14 @@ public class InlinableDeinit {
 @_fixed_layout
 public class InlineAlwaysDeinit {
  @inline(__always) deinit {
-    let ALLNOTYPECHECK_local = 1
-    let INLINENOTYPECHECK_local = 1
+    let NEVERTYPECHECK_local = 1


Didn't realise you could specify that arg multiple times, handy. Much nicer 👍

bnbarham · 2020-11-09T21:41:52Z

test/Frontend/skip-function-bodies.swift

-// %FileCheck %s --check-prefixes CHECK,CHECK-ALL-ONLY < %t/Skip.all.sil
+// RUN: %target-swift-frontend -emit-sil -emit-sorted-sil -experimental-skip-non-inlinable-function-bodies -debug-forbid-typecheck-prefix NEVERTYPECHECK -debug-forbid-typecheck-prefix INLINENOTYPECHECK %s -o %t/Skip.noninlinable.sil
+// RUN: %target-swift-frontend -emit-sil -emit-sorted-sil -experimental-skip-non-inlinable-function-bodies-without-types -debug-forbid-typecheck-prefix NEVERTYPECHECK -debug-forbid-typecheck-prefix TYPESNOTYPECHECK %s -o %t/Skip.withouttypes.sil
+// RUN: %target-swift-frontend -emit-sil -emit-sorted-sil -O -experimental-skip-all-function-bodies -debug-forbid-typecheck-prefix NEVERTYPECHECK -debug-forbid-typecheck-prefix ALLNOTYPECHECK %s -o %t/Skip.all.sil


If you do make any more changes feel free to remove the -O as well, but doesn't matter either way.

bnbarham · 2020-11-09T21:44:17Z

test/Frontend/skip-function-bodies.swift

+      _blackHole("didSet body") // Do not check for "didSet body" as it's in
+                                // SIL but not the swiftinerface.


Could add a separate check for the swiftinterface?

I just checked this locally - didSet body isn't in the serialized SIL for me (CHECK fails, CHECK-NOT succeeds). Am I missing something here?

Also, just noticed typo - "swiftinerface" -> "swiftinterface"

Right, I see a few issues if I set it to CHECK. First, it fails to find the next string @_transparent method body, which is probably an ordering issue, the test succeeded because it had already skipped over that string when looking ahead for the CHECK-NOT. And then I see the string in Skip.noninlinable.sil but not in Skip.all.sil, is that expected?

Just to be clear on the terminology, I'd call this textual SIL or simply SIL, serialized SIL wouldn't be as readable.

Sigh, sorry - I rushed this part of the test. Should have double checked the generated textual SIL. The ordering looks a little weird, the willSet is between publicFunc and privateFunc and the didSet is between the transparent init and inlinable init. Assuming that's stable (?) they should both be moved.

It's definitely expected that it's skipped for -experimental-skip-all-function-bodies - that option just skips outputting SIL entirely. I had thought it would be skipped for non-inlinable as well, but that looks like a misunderstanding on my part.

FWIW, the old "skip-noninline-function-bodies.swift" test had a CHECK prefix for SIL vs interface. I had removed it as it was being used, but this looks like a case for it.

xymus · 2020-11-11T21:31:39Z

@swift-ci Please smoke test Linux platform

This frontend flag can be used as an alternative to -experimental-skip-non-inlinable-function-bodies that doesn’t skip functions defining nested types. We want to keep these types as they are used by LLDB. Other functions ares safe to skip parsing and type-checking. rdar://71130519

xymus · 2020-11-12T22:29:17Z

@swift-ci Please smoke test

bnbarham · 2020-11-12T22:35:03Z

Thanks for fixing that test up!

xymus · 2020-11-12T23:21:28Z

@bnbarham Of course, and thanks for that previous PR, it made this one much cleaner.

xymus · 2020-11-13T01:24:08Z

@swift-ci Please smoke test macOS platform

Uses the newer `-experimental-skip-non-inlinable-function-bodies-without-types` which was introduced here: swiftlang/swift#34612. This should improve LLDB usage in some cases. When using WMO, it has two downsides in Swift 5.5, both introduced by swiftlang/swift#38939: - The swiftmodules will have swiftdeps info embedded in them, which is only needed for incremental compilation - Interface hashing is enabled, which again is only needed for incremental compilation This is because the swift compiler only expects `-experimental-skip-non-inlinable-function-bodies-without-types` to be used with the new `emit-module-separately` incremental build.

xymus requested review from bnbarham and nkcsgexi November 6, 2020 18:32

nkcsgexi approved these changes Nov 6, 2020

View reviewed changes

xymus force-pushed the dont-skip-nested-types branch from edf4098 to 526adc6 Compare November 6, 2020 21:05

xymus mentioned this pull request Nov 6, 2020

Preserve nested type info in swiftmodule files generated by the emit-module job swiftlang/swift-driver#350

Merged

bnbarham approved these changes Nov 9, 2020

View reviewed changes

xymus added 2 commits November 12, 2020 14:22

[Tests] Fix and refactor skip-function-bodies.swift

92d2c23

xymus force-pushed the dont-skip-nested-types branch from c4d9984 to 02c1343 Compare November 12, 2020 22:28

xymus merged commit b72b0c3 into swiftlang:main Nov 13, 2020

xymus mentioned this pull request Dec 4, 2020

[SIL] Don't verify functions that were skipped #34948

Merged

xymus deleted the dont-skip-nested-types branch August 25, 2021 17:46

brentleyjones mentioned this pull request Oct 14, 2021

Use newer split module flag with Swift 5.4 bazelbuild/rules_swift#699

Draft

		// %FileCheck %s --check-prefixes CHECK,CHECK-NONINLINE-ONLY < %t/Skip.noninlinable.sil
		// %FileCheck %s --check-prefixes CHECK,CHECK-ALL-ONLY < %t/Skip.all.sil

		_blackHole("didSet body") // Do not check for "didSet body" as it's in
		// SIL but not the swiftinerface.

[Sema] Add flag to optimize building swiftmodule files preserving type info for LLDB #34612

[Sema] Add flag to optimize building swiftmodule files preserving type info for LLDB #34612

Uh oh!

Conversation

xymus commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xymus commented Nov 6, 2020

Uh oh!

nkcsgexi left a comment

Choose a reason for hiding this comment

Uh oh!

xymus commented Nov 6, 2020

Uh oh!

xymus commented Nov 6, 2020

Uh oh!

swift-ci commented Nov 6, 2020

Uh oh!

swift-ci commented Nov 7, 2020

Uh oh!

xymus commented Nov 9, 2020

Uh oh!

xymus commented Nov 9, 2020

Uh oh!

bnbarham left a comment

Choose a reason for hiding this comment

Uh oh!

bnbarham Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

bnbarham Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

bnbarham Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

bnbarham Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

bnbarham Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

xymus Nov 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bnbarham Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

xymus commented Nov 11, 2020

Uh oh!

xymus commented Nov 12, 2020

Uh oh!

bnbarham commented Nov 12, 2020

Uh oh!

xymus commented Nov 12, 2020

Uh oh!

xymus commented Nov 13, 2020

Uh oh!

Uh oh!

xymus commented Nov 6, 2020 •

edited

Loading

xymus Nov 10, 2020 •

edited

Loading