[allocbox-to-stack] Fix an ossa bug in PromotedParamCloner. #34888
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
For those who are unfamiliar, alloc-box-to-stack while generally not interprocedural, will look one level into the callgraph to see if a partial_apply that captures a box really needs to capture the box due to an escape. If not, allocbox-to-stack clones the closure with the address inside the box being passed instead of the box itself. This can then allow us to promote the box from the heap to the stack. What went wrong here is that in OSSA, this promoted param cloner drops copy_value, destroy_value, and project_box on the given box. Both the copy_value and destroy_value cases correctly looked through copy_values, but when porting, the author forgot to handle project_box as well. This then caused the cloner to assert since: 1. The project_box in the original function had a copy_value operand. 2. When we visited that copy_value, we saw it was for the box, so we dropped the copy_value and did not add it to the cloner's Value -> op(Value) map. 3. Then when the cloner tried to create op(project_box), it tries to lookup the value associated with the copy_value that is the project_box's operand... but we don't have any such value due to (2). =><=. The test change exercises this code path by adding a (project_box (copy_value)) to one of the allocbox to stack tests.
|
@swift-ci test |
|
Build failed |
|
@swift-ci test linux platform |
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.
For those who are unfamiliar, alloc-box-to-stack while generally not
interprocedural, will look one level into the callgraph to see if a
partial_apply that captures a box really needs to capture the box due to an
escape. If not, allocbox-to-stack clones the closure with the address inside the
box being passed instead of the box itself. This can then allow us to promote
the box from the heap to the stack.
What went wrong here is that in OSSA, this promoted param cloner drops
copy_value, destroy_value, and project_box on the given box. Both the copy_value
and destroy_value cases correctly looked through copy_values, but when porting,
the author forgot to handle project_box as well. This then caused the cloner to
assert since:
The project_box in the original function had a copy_value operand.
When we visited that copy_value, we saw it was for the box, so we dropped the
copy_value and did not add it to the cloner's Value -> op(Value) map.
Then when the cloner tried to create op(project_box), it tries to lookup the
value associated with the copy_value that is the project_box's operand... but we
don't have any such value due to (2). =><=.
The test change exercises this code path by adding a (project_box (copy_value))
to one of the allocbox to stack tests.