[RISC-V] Fix Shuffling Thunks part 2 #90707
Conversation
That script was removed in commit 7d6b73e.
* Don't offset ShuffleEntry's by 10, because that is likely to go out of bounds. * Base for floating argument regs (fa) is 10 on RISC-V, not 1. * Src and dst registers in emit fld were reversed * Restore arguments stashed in temporary registers
* Remove unused Emit* methods * Centralize instruction creation in *TypeInstr functions to limit the potential for mistakes in hard-coded shifts, masks, etc * Replace Emit32 calls with hard-coded opcodes with proper Emit* names.
ghost
added
the
community-contribution
| @@ -1276,7 +1276,6 @@ else | |||
| fi | |||
|
|
|||
| scriptPath=$(dirname $0) | |||
| ${scriptPath}/setup-stress-dependencies.sh --arch=$ARCH --outputDir=$coreOverlayDir | |||
There was a problem hiding this comment.
Why do you remove this part in the script?
There was a problem hiding this comment.
Because setup-stress-dependencies.sh has been removed, details in commit message.
|
|
||
| void EmitCallRegister(IntReg reg); | ||
| void EmitRet(IntReg reg); |
There was a problem hiding this comment.
Probably, you need to remove EmitRet() declaration too since removed implementation
There was a problem hiding this comment.
Good catch, will add to next PR.
| } | ||
|
|
||
| void StubLinkerCPU::EmitCmpReg(IntReg Xn, IntReg Xm) | ||
| // Instruction types as per RISC-V Spec, Chapter 24 RV32/64G Instruction Set Listings | ||
| static unsigned ITypeInstr(unsigned opcode, unsigned funct3, unsigned rd, unsigned rs1, int imm12) |
There was a problem hiding this comment.
*TypeInstr functions are very useful, I think they should be available in JIT as well
There was a problem hiding this comment.
Can be moved. Since I'm fairly new to the codebase, what's a good place to put code shared between VM and JIT? utilcode?
There's some more redundancy between the JIT and VM emitter code, e.g. isValid*imm*(), that would be worth factoring out to a common place.
There was a problem hiding this comment.
src/coreclr/utilcode/util.cpp looks more suitable.
@jkotas any thoughts?
There was a problem hiding this comment.
The duplication of instruction encoding helpers is very small and it is there on all platforms. I do not think that it is worth fixing. The instruction encoding support in the JIT has to support nearly all instruction. The stublinker needs to be able to encode just a few instructions and so it can be very simple.
tomeksowi
changed the title
ghost
locked as resolved and limited conversation to collaborators
Fixes TestMRB6 from
./Regressions/coreclr/GitHub_16833/Test16833/Test16833.shThis validates the floating registers shuffling adapted from loongarch64.
Apart from the fix, I cleaned up the code around
Emit*a bit.Part of #84834
cc @wscho77 @HJLeee @JongHeonChoi @t-mustafin @alpencolt @gbalykov @clamp03 @jkotas