-
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
Update coredistools #96286
Update coredistools #96286
Conversation
Update to the 1.2.0 version of the coredistools package. Also, implement a basic late disassembler for RyuJIT based on coredistools (previously, this depended on closed-source msvcdis). Various changes: 1. Remove unused AddressMap in SPMI. Removed a bunch of cases in the NearDiffer that were using it. 2. Change arm64 emitter unit tests to not require `verbose` 3. Always build in the late disassembler under DEBUG. It's only used when `JitLateDisasm` is set. 4. Implement new arm32 callback mechanism for movw/movt handling in the NearDiffer.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsUpdate to the 1.2.0 version of the coredistools package. Also, implement a basic late disassembler for RyuJIT based on Various changes:
|
1. Fix Risc-V build: only build LATE_DISASM for platforms where coredistool support exists. 2. Defer load and initialization of coredistools.dll library for LATE_DISASM until we actually need it. 3. For arm64, in late disasm, if there is a failure, try to recover by skipping one 4-byte instruction.
/azp run runtime-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-coreclr gcstress0x3-gcstress0xc |
Azure Pipelines successfully started running 1 pipeline(s). |
gcstress pipeline failure is #96364 |
@jakobbotsch @dotnet/jit-contrib PTAL Note this includes changes to work with the 1.4.0 coredistools, but it will also work with the existing 1.1.0 coredistools. Changing everyone to use 1.4.0 is #96291. This still has some problem with linux-x64 gcstress. The rebuild of cordistools to 1.4.0 is dotnet/jitutils#370. This build still fails in CI with linux-x64 and linux-arm64, but those have been manually built and packaged into an uploaded 1.4.0 coredistools. |
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.
Just reviewed changes in jit
folder and they looks good except for few questions/suggestions.
In particular, only load the coredistools library once, not once per function to disassemble.
superpmi pipeline failures are due to missing osx-arm64 MCH files due to new spmi collection |
/azp run runtime-coreclr superpmi-replay, runtime-coreclr superpmi-diffs |
Azure Pipelines successfully started running 2 pipeline(s). |
@@ -37,6 +38,8 @@ | |||
|
|||
/*****************************************************************************/ | |||
|
|||
#ifdef USE_MSVCDIS |
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.
Do we ever use MSVCDIS? Can we remove all of this?
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.
We currently don't, but we could resurrect the code and use it. It shouldn't be too hard to do so, and there might be benefits. The only downside is it's not open source. So I'd prefer to just leave it.
#if 0 | ||
// We might want to do something similar for mov/movk/movk/movk sequences on Arm64. The following code was | ||
// previously used, but currently is not active. |
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.
Delete the code?
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'm not convinced yet that we don't need (or will never need) this, so I think it's worth leaving.
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
Update to the 1.4.0 version of the coredistools package.
Also, implement a basic late disassembler for RyuJIT based on
coredistools (previously, this depended on closed-source msvcdis).
Various changes:
the NearDiffer that were using it.
verbose
used when
JitLateDisasm
is set.in the NearDiffer.