-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[AddressLowering] Handle try_apply with enum'd return. #61963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
nate-chandler
wants to merge
17
commits into
swiftlang:main
from
nate-chandler:opaque-values/1/20221107
Closed
[AddressLowering] Handle try_apply with enum'd return. #61963
nate-chandler
wants to merge
17
commits into
swiftlang:main
from
nate-chandler:opaque-values/1/20221107
Conversation
This file contains hidden or 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
855e007
to
bc5f223
Compare
Fixes getSpecializationLevelRecursive to handle recursive manglings caused by interleaving CapturePropagation and ClosureSpecialize passes. For some reason, only the first closure parameter was checked for recursion. We need to handle patterns like this: kind=FunctionSignatureSpecialization kind=SpecializationPassID, index=3 kind=FunctionSignatureSpecializationParam kind=FunctionSignatureSpecializationParam kind=FunctionSignatureSpecializationParamKind, index=0 kind=FunctionSignatureSpecializationParamPayload, text="$s4test10ExpressionO8contains5whereS3bXE_tFSbACXEfU_S2bXEfU_36$s4test12IndirectEnumVACycfcS2bXEfU_Tf3npf_n" I fixed the logic so we now check for recursion on all closure parameters and bail out on unrecognized mangling formats. For reference, see summary.sil in Infinitely recursive closure specialization swiftlang#61955 swiftlang#61955 Fixes rdar://101589190 (Swift Compiler hangs when building this code for release)
The test's correctness doesn't depend on the exact sequence of instructions. Use IDs to allow for other instructions to appear.
When rewriting uses, it is possible for new uses of a value to be created, as when a debug_value instruction is created when a store instruction is deleted. Ensure that all uses are rewritten by adding all uses to the worklist of uses after rewriting each use.
Resizing a SmallVector of SILValues fills it with invalid SILValues. THe intent is to reserve space for forthcoming values.
The filterDeadArgs function takes a list of dead argument indices--ordered from least to greatest--a list of original arguments, and produces a list of arguments excluding the arguments at those dead indices. It does that by iterating from 0 to size(originalArguments) - 1, adding the original argument at that index to the list of new arguments, so long as the index that of a dead argument. To avoid doing lookups into a set, this relies on the dead arguments being ordered ascending. There is an interator into the dead argument list that is incremented only when the current index is dead. When that iterator is at the end, dereferencing it just gives the size of the array of dead arguments. So in the case where the first argument is dead but no other arguments are, and there _are_ other arguments, the first argument would be skipped, and the second argument's index would be found to be equal to the dereferenced iterator (1). Previously, there was no check that the iterator was not at the end. The result was failing to add the second argument to the new list. And tripping an assertion failure. Here, it is checked that the iterator is not at the end.
When a block argument is a terminator result from a try_apply, use the ApplyRewriter to convert the try_apply. In the case where the result is stored into an enum, with this change, the init_enum_data_addr instruction is created prior to the try_apply which is necessary in order for it to be passed as an argument to the try_apply.
If a switch_enum instruction (1) exhaustively handles all cases, there is no default case or block corresponding to it. If (2) it handles all cases but one, the default case corresponds to the unique unhandled case. Otherwise, (3) the default case corresponds to all the unhandled cases. The first two scenarios were already handled by address lowering. Here, handling is added for case (3). It is similar to what is already done for rewriting cases except that no unchecked_take_enum_data_addr must be created and that the argument is always address-only (being the same type as the operand of the switch_enum which is only being rewritten because it was address-only).
…roid, enable two more new ones, and set an executable_test Also, make some related changes, like updating a path in the Android doc, making sure the `unknown` vendor is always used, and using `CPU` instead of `CODEGENERATOR`.
Make sure we disable forwarding diagnostics from the underlying instance when building a swiftinterface in silencing errors mode.
During def rewriting, the def itself can be changed, for example to be a "dummy" load. In such cases, uses of the new def need to be rewritten, not uses of the original def.
bc5f223
to
93e5701
Compare
…/20221103 [AddressLowering] Some small fixes.
@swift-ci please test |
…ftinterface [ModuleInterface] Fix silencing errors mode in swiftinterface rebuild
[android][test] Fix two C++ Interop tests that were split off for Android and enable two more new ones
…/20221105 [AddressLowering] Handle multidefault switch_enum.
atrick
approved these changes
Nov 9, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…hang Fix infinite recursion in ClosureSpecialize
…/20221107 [AddressLowering] Handle try_apply with enum'd return.
Something went wrong here. The PR was merged at b2e34b2 , but remained open in GitHub's UI. Rebasing and pushing a branch without commits resulted in closing and automatically requesting review. Sorry for the noise! |
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.
When a block argument is a terminator result from a
try_apply
, use the ApplyRewriter to convert thetry_apply
.In the case where the result is stored into an enum, with this change, the
init_enum_data_addr
instruction is created prior to thetry_apply
, which is necessary in order for it to be passed as an argument to thetry_apply
.Based on #61950 .