Optimizer: replace PredictableMemoryAccessOptimizations with a "mandatory" redundant load elimination pass #79186
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
eeckstein
requested review from
jckarter,
zoecarver,
hyp,
egorzhdan,
Xazax-hun,
rjmccall,
ktoso and
adrian-prantl
as code owners
eeckstein
changed the title
eeckstein
requested review from
atrick,
meg-gupta and
nate-chandler
and removed request for
ktoso,
rjmccall,
hyp,
Xazax-hun,
adrian-prantl,
egorzhdan and
zoecarver
|
@swift-ci test |
|
@swift-ci apple silicon benchmark |
|
@swift-ci test source compatibility |
eeckstein
force-pushed
the
remove-predictable-memopt
branch
from
4f27c83 to
e171dd9
Compare
|
@swift-ci test |
…h was written by copy_addr For example: ``` %1 = alloc_stack $B copy_addr %0 to [init] %1 %3 = load [take] %1 dealloc_stack %1 ``` -> ``` %3 = load [copy] %0 ```
Memory effects of begin_access are only defined to prevent the optimizer moving loads and stores across a begin_access. But those memory effects are not relevant for RedundantLoadElimination
…emoved load If the memory location depends on something, insert a dependency for the loaded value: ``` %2 = mark_dependence %1 on %0 %3 = load %2 ``` -> ``` %2 = mark_dependence %1 on %0 // not needed anymore, can be removed eventually %3 = load %2 %4 = mark_dependence %3 on %0 // replace %3 with %4 ```
…n pass And make `eliminateRedundantLoads` callable from other optimizations
…yRedundantLoadElimination in the pass pipeline PredictableMemoryAccessOptimizations has become unmaintainable as-is. RedundantLoadElimination does (almost) the same thing as PredictableMemoryAccessOptimizations. It's not as powerful but good enough because PredictableMemoryAccessOptimizations is actually only needed for promoting integer values for mandatory constant propagation. And most importantly: RedundantLoadElimination does not insert additional copies which was a big problem in PredictableMemoryAccessOptimizations. Fixes rdar://142814676
…ead of `optimizeMemoryAccesses` to optimize redundant loads
Reported here: swiftlang#56221 rdar://72885279
eeckstein
force-pushed
the
remove-predictable-memopt
branch
from
e171dd9 to
d351d10
Compare
|
@swift-ci test |
|
@swift-ci smoke test windows |
3405691582
added a commit
to 3405691582/swift
that referenced
this pull request
Feb 23, 2025
That pr converted PredictableMemoryAccessOptimizations function pass to Swift. Both versions of this pass remove unreachable terminators at the end of apparent infinite loops that are guaranteed to eventually return. The stdlib has a number of instances where apparent infinite loops are used. When Swift host tools are available, so are the Swift function passes, and these unreachable terminators are removed as before. However, when the host tools are not available (e.g., using a configuration like the stdlib and we get a dataflow error manifesting as a "missing return". To extricate ourselves out of this situation, we have a few options. One option is determining that swiftlang#79186 marks the end of the road for the compiler to be bootstrapped from C++. However, that is somewhat unsatisfying, since there is a great reticence to accepting maintenance patches on previous compiler branches and leaves questions about how an older version of the "bootstrap" compiler should be kept up-to-date, since presumably as new platforms come online to bootstrap Swift, code changes to C++ would still be warranted. Alternatively, there is already a stated goal that the C++ compiler should not necessarily emit the most optimized code, and that the compiler built with Swift host tools would emit better optimized code. In this case, we would want to add an additional pass for the bootstrap compiler to just detect the apparent infinite loop case and make sure no errors occur. This is probably the correct thing to do, however, it is probably a slightly nontrivial undertaking. In the meantime, we need to ensure we can continue to bootstrap with the spot where the "missing return" error happens. This is not exactly the cleanest solution, but it does ensure the build is fixed again in this configuration.
|
Hi @eeckstein A SIL verification failure we started encountering in our internal app build seems to point to this PR: #79491 (comment) Thoughts? |
etcwilde
added a commit
to etcwilde/swift
that referenced
this pull request
Mar 5, 2025
PR 79186 (swiftlang#79186) moved one of the mandatory passes from the C++ implementation to the Swift implementation resulting in a compiler that is unable to build the standard library. The pass used to ensure that inaccessible control-flow positions after an infinite loop was marked with `unreachable` in SIL. Since the pass is no longer running, any function that returns a value that also has an infinite loop internally must place a fatalError after the infinite loop or it will fail to compile as the compiler will determine that the function does not return from all control flow paths even though some of the paths are unreachable.
etcwilde
added a commit
to etcwilde/swift
that referenced
this pull request
Mar 6, 2025
PR 79186 (swiftlang#79186) moved one of the mandatory passes from the C++ implementation to the Swift implementation resulting in a compiler that is unable to build the standard library. The pass used to ensure that inaccessible control-flow positions after an infinite loop was marked with `unreachable` in SIL. Since the pass is no longer running, any function that returns a value that also has an infinite loop internally must place a fatalError after the infinite loop or it will fail to compile as the compiler will determine that the function does not return from all control flow paths even though some of the paths are unreachable.
etcwilde
added a commit
to etcwilde/swift
that referenced
this pull request
Mar 6, 2025
PR 79186 (swiftlang#79186) moved one of the mandatory passes from the C++ implementation to the Swift implementation resulting in a compiler that is unable to build the standard library. The pass used to ensure that inaccessible control-flow positions after an infinite loop was marked with `unreachable` in SIL. Since the pass is no longer running, any function that returns a value that also has an infinite loop internally must place a fatalError after the infinite loop or it will fail to compile as the compiler will determine that the function does not return from all control flow paths even though some of the paths are unreachable.
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.
PredictableMemoryAccessOptimizations has become unmaintainable as-is.
RedundantLoadElimination does (almost) the same thing as PredictableMemoryAccessOptimizations.
It's not as powerful but good enough because PredictableMemoryAccessOptimizations is actually only needed for promoting integer values for mandatory constant propagation.
And most importantly: RedundantLoadElimination does not insert additional copies which was a big problem in PredictableMemoryAccessOptimizations.
rdar://142814676
Also fixes a complexity bug in PredictableMemoryAccessOptimizations.
#56221
rdar://72885279