JIT: Stop reporting FP locals as being on the x87 stack for 32-bit platforms #98580
Conversation
…atforms We never store any locals on the x87 stack and haven't for a while. Also, the handling kicks in for arm32 as well which does not make sense.
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsWe never store any locals on the x87 stack and haven't for a while. Also, the handling kicks in for arm32 as well which does not make sense. cc @dotnet/jit-contrib @filipnavara
|
| case TYP_FLOAT: | ||
| case TYP_DOUBLE: | ||
| if (isFloatRegType(type)) | ||
| { | ||
| this->vlType = VLT_FPSTK; | ||
| this->vlFPstk.vlfReg = varDsc->GetRegNum(); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Just to confirm, this isn't necessary for ABI purposes right? Floating-point args still get returned in ST(0) for Windows (and I think Unix as well).
There was a problem hiding this comment.
I believe so, since this handling is around locals.
| // TODO-AMD64-Bug: ndp\clr\src\inc\corinfo.h has a definition of RegNum that only goes up to R15, | ||
| // so no XMM registers can get debug information. |
There was a problem hiding this comment.
This comment is still true (though RegNum is no longer is corinfo.h, apparently).
See CodeGen::checkICodeDebugInfo(). We set vlrReg and assume that our register number values are the same as the RegNum values. RegNum doesn't define the FP registers, but we still set it. Seems odd. I presume the debugger ignores it?
There was a problem hiding this comment.
Well the comment might be true, but I think the real problem is that the current RegNumber is actually GeneralPurposeRegNumber, and the value stored in vlReg.vlrReg for VLT_REG_FP is some FloatingPointRegNumber enum that doesn't exist (i.e. xmm0 gets index 0). The debugger handles it here for x64:
runtime/src/coreclr/debug/di/rsthread.cpp
Lines 8405 to 8411 in efe8b87
I think I'll need to add some similar handling for x86 and run the debugger tests as part of this PR.
There was a problem hiding this comment.
This needs to be checked in the managed ObjWriter too since it expects the JIT register number (ie. not zero based).
Diff results for #98580Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64MinOpts (-0.01% to +0.01%)
Throughput diffs for osx/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Throughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.01% to +0.00%)
Details here |
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
We never store any locals on the x87 stack and haven't for a while. Also, the handling kicks in for arm32 as well which does not make sense.
cc @dotnet/jit-contrib @filipnavara