-
Notifications
You must be signed in to change notification settings - Fork 5.3k
JIT: Seed stress modes by string hash #83771
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: Seed stress modes by string hash #83771
Conversation
Instead of using the numeric value of each stress mode inside the enum, calculate a hash based on the string name. This has the benefit that it won't change based on new members being added/reordered and reduces correlation of which stress modes get enabled together. Fix dotnet#83733
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsInstead of using the numeric value of each stress mode inside the enum calculate a hash based on the string name. This has the benefit that it won't change based on new members being added/reordered and reduces correlation of which stress modes get enabled together. Fix #83733
|
kunalspathak
left a comment
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
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Looks like this exposes some new stress related issue: Assertion failed '(emitThisGCrefRegs & regMask) == 0' in 'System.String:Concat[ushort](System.Collections.Generic.IEnumerable`1[ushort]):System.String' during 'Emit code' |
|
The above issue also hits in libraries-jitstress-random, I opened #83963 for it. Will convert this to a draft until we get a fix for that. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
With side effects that may go on top, it may be block morph's responsibility to set this flag. Also set the type of the reused GT_COMMA nodes; haven't actually seen any issues here, but seems prudent to do this. Fixes some stress issues seen over in dotnet#83771.
| if (verbose) | ||
| { | ||
| printf("\n\n*** JitStress: %ws ***\n\n", s_compStressModeNames[stressArea]); | ||
| printf("\n\n*** JitStress: %s ***\n\n", s_compStressModeNames[stressArea]); |
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.
This also fixes this printf for PAL (it was just printing as "JitStress: %ws" on non-Windows builds)
With side effects that may go on top, it may be block morph's responsibility to set this flag. Also set the type of the reused GT_COMMA nodes; haven't actually seen any issues here, but seems prudent to do this. Fixes some stress issues seen over in #83771.
|
Stress failures should be fixed by #84352, will rerun... |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Yay, stress is finally green. Going to remove the stress mode I pointed at in #85975 (comment) and then merge. |
Instead of using the numeric value of each stress mode inside the enum calculate a hash based on the string name. This has the benefit that it won't change based on new members being added/reordered and reduces correlation of which stress modes get enabled together.
Fix #83733