-
Notifications
You must be signed in to change notification settings - Fork 5.2k
SPMI: Include JIT-EE GUID changes in JIT baseline detection #112820
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
Conversation
fb67dba changed the JIT-EE GUID, but it includes no changes under src/coreclr/jit/*. The result is that jitrollingbuild built a JIT on that version but overrode the JIT on the parent commit 7060c62 with it. That means the JIT for 7060c62 now has a wrong JIT-EE GUID. Fix the baseline detection so that it includes changes in the JIT-EE GUID. Also log the GUIDs when there is a mismatch in SPMI.
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.
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
Files not reviewed (1)
- src/coreclr/tools/superpmi/superpmi/jitinstance.cpp: Language not supported
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
EgorBo
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.
I vaguely remember we hit this in the past, I though we fixed it
| # Enumerate the last 20 changes, starting with the baseline, that included JIT changes. | ||
| command = [ "git", "log", "--pretty=format:%H", baseline_hash, "-20", "--", "src/coreclr/jit/*" ] | ||
| # Enumerate the last 20 changes, starting with the baseline, that included JIT and JIT-EE GUID changes. | ||
| command = [ "git", "log", "--pretty=format:%H", baseline_hash, "-20", "--", "src/coreclr/jit/*", "src/coreclr/inc/jiteeversionguid.h" ] |
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.
does git log support multiple paths?
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.
Yep, it does
fb67dba changed the JIT-EE GUID, but it includes no changes under src/coreclr/jit/*. The result is that jitrollingbuild built a JIT on that version but overrode the JIT on the parent commit 7060c62 with it. That means the JIT for 7060c62 now has a wrong JIT-EE GUID.
Fix the baseline detection so that it includes changes in the JIT-EE GUID. Also log the GUIDs when there is a mismatch in SPMI.