-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
JIT: enable cross-block local assertion prop #94689
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Detailsnull
|
The failures above are for the release-mode debug info testing. With cross-block we remove the store at IL offset 0 and this causes the validation to fail. Going to just add another exemption here... I'm surprised something downstream doesn't catch this already (basically |
Looks like a bug has crept in, possibly from the JTRUE assertions. |
Latest diffs. |
/azp run runtime-jit-experimental, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-coreclr outerloop |
Azure Pipelines successfully started running 4 pipeline(s). |
jit-experimental failure was that the |
FYI @dotnet/jit-contrib Latest diffs. Main impact here is a code size improvement; TP results are mixed but "average" out to about zero. |
/azp run Fuzzlyn |
Azure Pipelines successfully started running 1 pipeline(s). |
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, very nice to see this enabled. I kicked off a Fuzzlyn run with it enabled.
I have perf lab data too but so far it's indistinguishable from noise; I almost suspect the feature is not getting enabled, |
The Fuzzlyn runs will hit #95318 so we'll probably have to do a bit digging to see if there is anything else. |
I only see the unexpected small type asserts. |
The number of examples found does not match up with the number of those assertion occurrences... But I'm not totally sure whether those counts are accurate or not given the number of examples found. |
#95249 was merged now so I'm going to rerun Fuzzlyn to be sure. |
/azp run Fuzzlyn |
Azure Pipelines successfully started running 1 pipeline(s). |
Do those runs merge up or do we need a new merge from main? |
All good. |
Collated set of improvements (lower is better) as of 12-12-23. 134 benchmarks improved.
|
Enable cross-block assertion prop by default.