[EDS] Emit resilient conditional runtime records in Swift #73060
+171
−156
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NuriAmari
commented
Apr 16, 2024
| @@ -640,10 +640,15 @@ emitGlobalList(IRGenModule &IGM, ArrayRef<llvm::WeakTrackingVH> handles, | |||
| IGM.addUsedGlobal(var); | |||
| } | |||
|
|
|||
| // Keep classes and categories around for potential ObjC interop. | |||
| if (IGM.IRGen.Opts.SafeConditionalRuntimeRecords && | |||
There was a problem hiding this comment.
At some point this was found to be unsafe, though I wasn't around when the issue was discovered. I've thought about removing this restriction a few times, but it seems unsafe to me. IIUC objc_classes and objc_categories sections are read by the runtime, and so usages would not be detectable statically.
NuriAmari
commented
Apr 16, 2024
| @@ -1,47 +0,0 @@ | |||
| // Tests that with -conditional-runtime-records, IRGen marks class, struct, | |||
There was a problem hiding this comment.
This test is replaced by the ConditionIRGen test
This was referenced Apr 17, 2024
|
@swift-ci Please test apple/llvm-project#8588 |
|
@swift-ci Please test |
NuriAmari
commented
Apr 17, 2024
Conditional runtime records is an existing frontend flag that prompts
the Swift frontend to emit metadata that is designed to communicate
to dead stripping optimization passes when certain runtime
records are eligible for stripping, based on the liveness of other
symbols.
This metadata takes the following form in LLVM IR, communicating
the target global may be stripped if either dep_1, or dep_2 is dead:
```
!1 = !{ptr @target, i32 1, !{ptr @dep_1, ptr @dep_2}}
```
The issue with this metadata is that it is rather brittle. Globals
referenced in this metadata can be deleted, replacing references
to the globals in metadata with nullptr values. A nullptr value
does not necessarily indicate a dead dependency.
How the liveness of a dependency is to be interpretted, depends
on the linkage of the dependency, and if it is declared or
defined within the module.
With just a nullptr value, one cannot identify which case
we are in, and thus must conservatively treat such values as live
depdendencies.
One solution to this problem is to audit the optimization pipeline
for passes that invalidate the metadata in this way, and prevent
the invalidation. This doesn't scale however, and can break at any time,
as metadata isn't guaranteed to be maintained by LLVM passes.
Instead, we emit real LLVM globals, similar to llvm.used. They are
stored in `llvm.compiler.used`, and so will be preserved until
the linker. At this point they will be automatically dropped by
any linker using dead stripping.
Until that point, they will hold their dependencies live, automatically
protected from invalidation, as they are data structures like any other.
Since we are emitting data, these structures can also be interpretted
by the linker, a feature the metadata did not support.
This patch replaces emitting the old metadata, with the new format. I
have been referring to each such record as a "conditionally live
record", and emitting them into a special section for easy
identification.
This patch also adds a new front-end flags,
-safe-conditional-runtime-records, that emits fewer conditionally
live records, avoiding a case with Objective-C class records that
may be unsafe with Objective-C interop. The existing flag
still emits the same records, just in the new format.
NuriAmari
force-pushed
the
resilient-conditional-runtime-records
branch
from
d66e91b
to
07ce9c2
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
Corresponding forum post: https://forums.swift.org/t/rfc-conditional-runtime-records-revamped/71326 |
NuriAmari
requested review from
tshortli,
hborla,
slavapestov and
xedin
as code owners
|
@swift-ci Please test |
|
Compat suite failures look unrelated. |
aschwaighofer
approved these changes
Apr 25, 2024
|
Thanks for the review @aschwaighofer , do you mind taking a look at the LLVM side: apple/llvm-project#8588 ? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Conditional runtime records is an existing frontend flag that prompts the Swift frontend to emit metadata that is designed to communicate to dead stripping optimization passes when certain runtime records are eligible for stripping, based on the liveness of other symbols.
This metadata takes the following form in LLVM IR, communicating the target global may be stripped if either dep_1, or dep_2 is dead:
The issue with this metadata is that it is rather brittle. Globals referenced in this metadata can be deleted, replacing references to the globals in metadata with nullptr values. A nullptr value does not necessarily indicate a dead dependency.
How the liveness of a dependency is to be interpretted, depends on the linkage of the dependency, and if it is declared or defined within the module.
With just a nullptr value, one cannot identify which case we are in, and thus must conservatively treat such values as live dependencies.
One solution to this problem is to audit the optimization pipeline for passes that invalidate the metadata in this way, and prevent the invalidation. This doesn't scale however, and can break at any time, as metadata isn't guaranteed to be maintained by LLVM passes.
Instead, we emit real LLVM globals, similar to llvm.used. They are stored in
llvm.compiler.used, and so will be preserved until the linker. At this point they will be automatically dropped by any linker using dead stripping.Until that point, they will hold their dependencies live, automatically protected from invalidation, as they are data structures like any other. Since we are emitting data, these structures can also be interpretted by the linker, a feature the metadata did not support.
This patch replaces emitting the old metadata, with the new format. I have been referring to each such record as a "conditionally live record", and emitting them into a special section for easy identification.