[RISC-V] Fix gc-related bugs in risc-v emitter #98226
Conversation
…dotnet#96741)"" This reverts commit ecc044d.
ghost
added
the
community-contribution
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThis PR fixes a bug related to the gc info update which caused a lot of tests to fail with crossgen2. Also a little code refactor Part of #84834
|
…com:Bajtazar/runtime into riscv-emit-designated-instr-crossgen2-fixes
Diff results for #98226Throughput diffsThroughput diffs for osx/arm64 ran on windows/x64MinOpts (-0.00% to +0.01%)
Throughput diffs for windows/arm64 ran on windows/x64MinOpts (-0.00% to +0.01%)
Details here |
src/coreclr/jit/emitriscv64.cpp
Outdated
| ssize_t format = immediate >> 8; | ||
| assertCodeLength(format, 3); |
There was a problem hiding this comment.
I cannot understand what it means. It looks assertCodeLength(format, 8) is ok. Could you please explain?
There was a problem hiding this comment.
Sorry for this bug, this functions have been synchronized with my latest changes, but the fence usage haven't been tested or used yet hence this bug was not noticed. I've wanted to check whether the format of the fence is equal to 0b0000 or 0b1000 since currently those are only allowed values. It seems that I've too eagerly replaced the assert((format & 0x7) == 0) with this line of code, although as I said this function is never called with the fence instruction yet so it wouldn't cause any regression. Thanks for pointing that out
|
LGTM |
This PR fixes a bug related to the gc info update which caused a lot of tests to fail with crossgen2. Also a little code refactor
Part of #84834
cc @wscho77 @HJLeee @clamp03 @JongHeonChoi @t-mustafin @gbalykov @viewizard @ashaurtaev @brucehoult @sirntar @yurai007 @tomeksowi