[JSC] Fix GetButterfly handling in DFG array allocation sinking #47364
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
|
EWS run on previous version of this PR (hash d98d524) |
|
EWS run on previous version of this PR (hash f0be5a6) |
https://bugs.webkit.org/show_bug.cgi?id=295191 rdar://154419571 Reviewed by NOBODY (OOPS!). When arrays are sunk and later materialized in DFG object allocation sinking, GetButterfly nodes that depend on those arrays were not properly updated to reference the materialized arrays, causing incorrect butterfly references. The fix tracks GetButterfly nodes as dependencies of array allocations and creates materialized GetButterfly nodes that reference the materialized arrays when arrays escape and need to be materialized.
|
EWS run on current version of this PR (hash 2c43384) |
| @@ -1030,14 +1030,17 @@ class ObjectAllocationSinkingPhase : public Phase { | |||
| Node* base = node->child1().node(); | |||
There was a problem hiding this comment.
Let's separate GetBufferfly and GetArrayLength handling, which makes code cleaner.
case GetButterfly {
...
break;
}
case GetArrayLength: {
...
break;
}
Comment on lines
+2329
to
+2333
| case GetButterfly: | ||
| // GetButterfly nodes will be removed during DCE since they become unused | ||
| // after materialization replacement. | ||
| break; | ||
|
|
There was a problem hiding this comment.
I think using PhantomGetButterfly would be simpler. Then, without a special handling of dependency, we can just keep it in the phantom tree of creation.
Comment on lines
+2202
to
+2203
| if (escapee->op() != GetButterfly) | ||
| populateMaterialization(block, materialization, escapee); |
There was a problem hiding this comment.
If we have PhantomGetButterfly, you do not need to have this.
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.
2c43384
2c43384