Fix for preventing copy-prop of foo.arguments to support the creation of arguments object during each access. #1326
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
… of arguments object during each access.
satheeshravi
force-pushed
the
StackArgsBugFix3
branch
from
11da226 to
22bb957
Compare
| } | ||
| originalPropertySym = sym->AsPropertySym(); | ||
|
|
||
| //Don't copy-prop for cases like foo.arguments |
Contributor
There was a problem hiding this comment.
Can you check if you can use argumentsEquivBv on Globopt to solve your issue? That bit vector is there for these kind of issues only- maybe we need to expand the usage of it. Basically, right now we use the bv to disable copy-prop or arguments across inlinees
Contributor
|
@satheeshravi Any update on this? |
Contributor
Author
|
I have been working on some other high pri work. I will get back to this soon. |
Contributor
Author
|
Will close this PR. There is a new fix for this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Premise:
foo.arguments should get a new copy of the arguments object everytime it is accessed (this change was in effect from my recent Stack Args change).
We were doing this right only when the instruction is profiled - by adding a ImplicitAccessor flag in the fldInfoFlags.
But this doesn't cover the cases where the instruction is not profiled (as in cases equivalent to forcenative or forcejitloopbody).
The property sym for arguments fld can be copy proped to the subsequent uses of it, which is wrong.
Fix:
Added a direct check in the OptSrc to detect this case and to disable copy prop for this field.
Test:
Will do a quick perf test and will start other tests.