-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Copy propagation tweaking #64378
Copy propagation tweaking #64378
Conversation
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsSome small CQ improvements in preparation for a zero-diff TP-oriented refactoring.
|
@dotnet/jit-contrib |
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
The type check is too conservative: it prevents partial definitions from being used in propagation: ``` LCL_FLD V00/1 [X] = { ... }; // Pushed on the stack as a def. USE LCL_VAR V01 // Has the same VN as V00/1, but the type // check prevented it from being replaced. ``` This new version is conservative too, but will do for now as we don't propagate on (most) partial uses. Another reason for this change is that in my upcoming refactoring of copy propagation (that will bring another 0.5% in TP gains), we will no longer have the "defNode" available.
Ordinarily, shadowed parameters would not be used for propagation anyway, because of the liveness check, but "this" bypasses that checks, and so was used, which is presumably not what we want. Regardless of that, it is also not profitable to propagate "this" in such a situation as it extends its live range and makes the RA unhappy.
873f338
to
f7be01c
Compare
src/coreclr/jit/compiler.h
Outdated
@@ -10858,6 +10858,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX | |||
public: | |||
struct ShadowParamVarInfo | |||
{ | |||
static const unsigned NO_SHADOW_COPY = UINT_MAX; |
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.
Nit: why not just use BAD_VAR_NUM
?
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.
I suppose no reason. Deleted it.
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.
Thanks again for your ongoing contributions.
Some small CQ improvements in preparation for a zero-diff TP-oriented refactoring.
Please refer to the individual commit messages for details.
Diffs.