-
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
Handle direct addresses for statics in IsFieldAddr
#64846
Handle direct addresses for statics in IsFieldAddr
#64846
Conversation
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsThis change improves We are expecting some nice CSE-related diffs. Part of
|
e310ee6
to
f38f503
Compare
IsFieldAddr
IsFieldAddr
e451dfd
to
32ed889
Compare
Diffs - obtained using the same SPMI trick as in #62632. We'll need a recollection once/if this change is merged. We see some size regressions in ARM64 collections, I've looked at some, nothing stands out (more CSEs -> not always better), and the overall PerfScore diff is positive: PerfScore diff for win-arm64, libraries.pmi
PerfScore diff for win-arm64, libraries_tests.pmi
Notably, there were cases in tests where we lost some CQ because constant propagation loses field sequences, and thus instead of uniform |
OSX x64 timeouts are not related (happening everywhere), Linux x64 failure is #61359. @dotnet/jit-contrib |
7615206
to
7f06e13
Compare
Allows this change to be tested via SPMI.
No diffs.
This reverts commit b17817e0354a63319256a0fa0d21b74c2cfbf781.
7f06e13
to
848169f
Compare
Latest asmdiff CI doesn't show any diffs. Do you know why? |
@kunalspathak That's expected. This change leads to lots of That's why I needed that field info commit to obtain the diffs, it caches the "is static" information from importation and avoids calling |
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. I will kick off a new SPMI collection.
Nice improvements in dotnet/perf-autofiling-issues#3834 |
@DrewScoggins this benchmark also improved with this PR, but did not autofile: |
This change improves
IsFieldAddr
to recognize statics with known constant addresses, such as those forobject
-typed fields.We are expecting some nice CSE-related diffs.
Part of
CLS_VAR
deletion.