Skip to content

Commit e3af00b

Browse files
Bajtazarclamp03
Bajtazar
and
clamp03
authored
[RISC-V] Fix gc-related bugs in risc-v emitter (#98226)
* [RISC-V] Fix mistakes in emitter * Revert "[RISC-V] Added designated output instruction emitters (#96741)" This reverts commit 77fd98c. * Revert "Revert "[RISC-V] Added designated output instruction emitters (#96741)"" This reverts commit ecc044d. * [RISC-V] Sync emitOutputIns with the latest ref branch * [RISC-V] Formatted code * [RISC-V] Fixes * [RISC-V] Fixed sign cast in assert code len * [RISC-V] Readed assert * [RISC-V] Fixed fence sanity check and removed fence_i --------- Co-authored-by: Dong-Heon Jung <[email protected]>
1 parent 4d69ab7 commit e3af00b

File tree

2 files changed

+89
-67
lines changed

2 files changed

+89
-67
lines changed

‎src/coreclr/jit/emitriscv64.cpp‎

Lines changed: 86 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2123,7 +2123,7 @@ unsigned emitter::emitOutput_Instr(BYTE* dst, code_t code) const
21232123
return sizeof(code_t);
21242124
}
21252125

2126-
static inline void assertCodeLength(unsigned code, uint8_t size)
2126+
static inline void assertCodeLength(size_t code, uint8_t size)
21272127
{
21282128
assert((code >> size) == 0);
21292129
}
@@ -2298,7 +2298,9 @@ static inline void assertCodeLength(unsigned code, uint8_t size)
22982298

22992299
static constexpr unsigned kInstructionOpcodeMask = 0x7f;
23002300
static constexpr unsigned kInstructionFunct3Mask = 0x7000;
2301+
static constexpr unsigned kInstructionFunct5Mask = 0xf8000000;
23012302
static constexpr unsigned kInstructionFunct7Mask = 0xfe000000;
2303+
static constexpr unsigned kInstructionFunct2Mask = 0x06000000;
23022304

23032305
#ifdef DEBUG
23042306

@@ -2338,34 +2340,44 @@ static constexpr unsigned kInstructionFunct7Mask = 0xfe000000;
23382340
assert(isGeneralRegisterOrR0(rs1));
23392341
assert(isGeneralRegisterOrR0(rs2));
23402342
break;
2341-
case INS_fadd_s:
2342-
case INS_fsub_s:
2343-
case INS_fmul_s:
2344-
case INS_fdiv_s:
23452343
case INS_fsgnj_s:
23462344
case INS_fsgnjn_s:
23472345
case INS_fsgnjx_s:
23482346
case INS_fmin_s:
23492347
case INS_fmax_s:
2350-
case INS_feq_s:
2351-
case INS_flt_s:
2352-
case INS_fle_s:
2353-
case INS_fadd_d:
2354-
case INS_fsub_d:
2355-
case INS_fmul_d:
2356-
case INS_fdiv_d:
23572348
case INS_fsgnj_d:
23582349
case INS_fsgnjn_d:
23592350
case INS_fsgnjx_d:
23602351
case INS_fmin_d:
23612352
case INS_fmax_d:
2353+
assert(isFloatReg(rd));
2354+
assert(isFloatReg(rs1));
2355+
assert(isFloatReg(rs2));
2356+
break;
2357+
case INS_feq_s:
23622358
case INS_feq_d:
23632359
case INS_flt_d:
2360+
case INS_flt_s:
2361+
case INS_fle_s:
23642362
case INS_fle_d:
2365-
assert(isFloatReg(rd));
2363+
assert(isGeneralRegisterOrR0(rd));
23662364
assert(isFloatReg(rs1));
23672365
assert(isFloatReg(rs2));
23682366
break;
2367+
case INS_fmv_w_x:
2368+
case INS_fmv_d_x:
2369+
assert(isFloatReg(rd));
2370+
assert(isGeneralRegisterOrR0(rs1));
2371+
assert(rs2 == 0);
2372+
break;
2373+
case INS_fmv_x_d:
2374+
case INS_fmv_x_w:
2375+
case INS_fclass_s:
2376+
case INS_fclass_d:
2377+
assert(isGeneralRegisterOrR0(rd));
2378+
assert(isFloatReg(rs1));
2379+
assert(rs2 == 0);
2380+
break;
23692381
default:
23702382
NO_WAY("Illegal ins within emitOutput_RTypeInstr!");
23712383
break;
@@ -2377,6 +2389,7 @@ static constexpr unsigned kInstructionFunct7Mask = 0xfe000000;
23772389
{
23782390
switch (ins)
23792391
{
2392+
case INS_mov:
23802393
case INS_jalr:
23812394
case INS_lb:
23822395
case INS_lh:
@@ -2392,7 +2405,6 @@ static constexpr unsigned kInstructionFunct7Mask = 0xfe000000;
23922405
case INS_lwu:
23932406
case INS_ld:
23942407
case INS_addiw:
2395-
case INS_fence_i:
23962408
case INS_csrrw:
23972409
case INS_csrrs:
23982410
case INS_csrrc:
@@ -2427,6 +2439,15 @@ static constexpr unsigned kInstructionFunct7Mask = 0xfe000000;
24272439
assert(rs1 < 32);
24282440
assert((opcode & kInstructionFunct7Mask) == 0);
24292441
break;
2442+
case INS_fence:
2443+
{
2444+
assert(rd == REG_ZERO);
2445+
assert(rs1 == REG_ZERO);
2446+
ssize_t format = immediate >> 8;
2447+
assert((format == 0) || (format == 0x8));
2448+
assert((opcode & kInstructionFunct7Mask) == 0);
2449+
}
2450+
break;
24302451
default:
24312452
NO_WAY("Illegal ins within emitOutput_ITypeInstr!");
24322453
break;
@@ -2867,7 +2888,7 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im
28672888
if (id->idReg2())
28682889
{
28692890
// special for INT64_MAX or UINT32_MAX
2870-
dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, 0xfff);
2891+
dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask(12));
28712892
const ssize_t shiftValue = (immediate == INT64_MAX) ? 1 : 32;
28722893
dst += emitOutput_ITypeInstr(dst, INS_srli, reg1, reg1, shiftValue);
28732894
}
@@ -2881,10 +2902,10 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im
28812902

28822903
BYTE* emitter::emitOutputInstr_OptsI32(BYTE* dst, ssize_t immediate, regNumber reg1)
28832904
{
2884-
ssize_t upperWord = UpperWordOfDoubleWord(immediate);
2905+
const ssize_t upperWord = UpperWordOfDoubleWord(immediate);
28852906
dst += emitOutput_UTypeInstr(dst, INS_lui, reg1, UpperNBitsOfWordSignExtend<20>(upperWord));
28862907
dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, reg1, LowerNBitsOfWord<12>(upperWord));
2887-
ssize_t lowerWord = LowerWordOfDoubleWord(immediate);
2908+
const ssize_t lowerWord = LowerWordOfDoubleWord(immediate);
28882909
dst += emitOutput_ITypeInstr(dst, INS_slli, reg1, reg1, 11);
28892910
dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, reg1, LowerNBitsOfWord<11>(lowerWord >> 21));
28902911
dst += emitOutput_ITypeInstr(dst, INS_slli, reg1, reg1, 11);
@@ -2899,39 +2920,37 @@ BYTE* emitter::emitOutputInstr_OptsRc(BYTE* dst, const instrDesc* id, instructio
28992920
assert(id->idAddr()->iiaIsJitDataOffset());
29002921
assert(id->idGCref() == GCT_NONE);
29012922

2902-
int dataOffs = id->idAddr()->iiaGetJitDataOffset();
2923+
const int dataOffs = id->idAddr()->iiaGetJitDataOffset();
29032924
assert(dataOffs >= 0);
29042925

2905-
ssize_t immediate = emitGetInsSC(id);
2926+
const ssize_t immediate = emitGetInsSC(id);
29062927
assert((immediate >= 0) && (immediate < 0x4000)); // 0x4000 is arbitrary, currently 'imm' is always 0.
29072928

2908-
unsigned offset = static_cast<unsigned>(dataOffs + immediate);
2929+
const unsigned offset = static_cast<unsigned>(dataOffs + immediate);
29092930
assert(offset < emitDataSize());
29102931

2911-
*ins = id->idIns();
2912-
regNumber reg1 = id->idReg1();
2932+
*ins = id->idIns();
2933+
const regNumber reg1 = id->idReg1();
29132934

29142935
if (id->idIsReloc())
29152936
{
2916-
return emitOutputInstr_OptsRcReloc(dst, ins, reg1);
2937+
return emitOutputInstr_OptsRcReloc(dst, ins, offset, reg1);
29172938
}
29182939
return emitOutputInstr_OptsRcNoReloc(dst, ins, offset, reg1);
29192940
}
29202941

2921-
BYTE* emitter::emitOutputInstr_OptsRcReloc(BYTE* dst, instruction* ins, regNumber reg1)
2942+
BYTE* emitter::emitOutputInstr_OptsRcReloc(BYTE* dst, instruction* ins, unsigned offset, regNumber reg1)
29222943
{
2923-
ssize_t immediate = emitConsBlock - dst;
2924-
assert(immediate > 0);
2925-
assert((immediate & 0x03) == 0);
2944+
const ssize_t immediate = (emitConsBlock - dst) + offset;
2945+
assert((immediate > 0) && ((immediate & 0x03) == 0));
29262946

2927-
regNumber rsvdReg = codeGen->rsGetRsvdReg();
2947+
const regNumber rsvdReg = codeGen->rsGetRsvdReg();
29282948
dst += emitOutput_UTypeInstr(dst, INS_auipc, rsvdReg, UpperNBitsOfWordSignExtend<20>(immediate));
29292949

29302950
instruction lastIns = *ins;
29312951

29322952
if (*ins == INS_jal)
29332953
{
2934-
assert(isGeneralRegister(reg1));
29352954
*ins = lastIns = INS_addi;
29362955
}
29372956
dst += emitOutput_ITypeInstr(dst, lastIns, reg1, rsvdReg, LowerNBitsOfWord<12>(immediate));
@@ -2940,12 +2959,12 @@ BYTE* emitter::emitOutputInstr_OptsRcReloc(BYTE* dst, instruction* ins, regNumbe
29402959

29412960
BYTE* emitter::emitOutputInstr_OptsRcNoReloc(BYTE* dst, instruction* ins, unsigned offset, regNumber reg1)
29422961
{
2943-
ssize_t immediate = reinterpret_cast<ssize_t>(emitConsBlock) + offset;
2944-
assert((immediate >> 40) == 0);
2945-
regNumber rsvdReg = codeGen->rsGetRsvdReg();
2962+
const ssize_t immediate = reinterpret_cast<ssize_t>(emitConsBlock) + offset;
2963+
assertCodeLength(static_cast<size_t>(immediate), 40);
2964+
const regNumber rsvdReg = codeGen->rsGetRsvdReg();
29462965

2947-
instruction lastIns = (*ins == INS_jal) ? (*ins = INS_addi) : *ins;
2948-
UINT32 high = immediate >> 11;
2966+
const instruction lastIns = (*ins == INS_jal) ? (*ins = INS_addi) : *ins;
2967+
const UINT32 high = immediate >> 11;
29492968

29502969
dst += emitOutput_UTypeInstr(dst, INS_lui, rsvdReg, UpperNBitsOfWordSignExtend<20>(high));
29512970
dst += emitOutput_ITypeInstr(dst, INS_addi, rsvdReg, rsvdReg, LowerNBitsOfWord<12>(high));
@@ -2959,9 +2978,8 @@ BYTE* emitter::emitOutputInstr_OptsRl(BYTE* dst, instrDesc* id, instruction* ins
29592978
insGroup* targetInsGroup = static_cast<insGroup*>(emitCodeGetCookie(id->idAddr()->iiaBBlabel));
29602979
id->idAddr()->iiaIGlabel = targetInsGroup;
29612980

2962-
regNumber reg1 = id->idReg1();
2963-
assert(isGeneralRegister(reg1));
2964-
ssize_t igOffs = targetInsGroup->igOffs;
2981+
const regNumber reg1 = id->idReg1();
2982+
const ssize_t igOffs = targetInsGroup->igOffs;
29652983

29662984
if (id->idIsReloc())
29672985
{
@@ -2974,7 +2992,7 @@ BYTE* emitter::emitOutputInstr_OptsRl(BYTE* dst, instrDesc* id, instruction* ins
29742992

29752993
BYTE* emitter::emitOutputInstr_OptsRlReloc(BYTE* dst, ssize_t igOffs, regNumber reg1)
29762994
{
2977-
ssize_t immediate = (emitCodeBlock - dst) + igOffs;
2995+
const ssize_t immediate = (emitCodeBlock - dst) + igOffs;
29782996
assert((immediate & 0x03) == 0);
29792997

29802998
dst += emitOutput_UTypeInstr(dst, INS_auipc, reg1, UpperNBitsOfWordSignExtend<20>(immediate));
@@ -2984,11 +3002,11 @@ BYTE* emitter::emitOutputInstr_OptsRlReloc(BYTE* dst, ssize_t igOffs, regNumber
29843002

29853003
BYTE* emitter::emitOutputInstr_OptsRlNoReloc(BYTE* dst, ssize_t igOffs, regNumber reg1)
29863004
{
2987-
ssize_t immediate = reinterpret_cast<ssize_t>(emitCodeBlock) + igOffs;
2988-
assert((immediate >> (32 + 20)) == 0);
3005+
const ssize_t immediate = reinterpret_cast<ssize_t>(emitCodeBlock) + igOffs;
3006+
assertCodeLength(static_cast<size_t>(immediate), 32 + 20);
29893007

2990-
regNumber rsvdReg = codeGen->rsGetRsvdReg();
2991-
ssize_t upperSignExt = UpperWordOfDoubleWordDoubleSignExtend<32, 52>(immediate);
3008+
const regNumber rsvdReg = codeGen->rsGetRsvdReg();
3009+
const ssize_t upperSignExt = UpperWordOfDoubleWordDoubleSignExtend<32, 52>(immediate);
29923010

29933011
dst += emitOutput_UTypeInstr(dst, INS_lui, rsvdReg, UpperNBitsOfWordSignExtend<20>(immediate));
29943012
dst += emitOutput_ITypeInstr(dst, INS_addi, rsvdReg, rsvdReg, LowerNBitsOfWord<12>(immediate));
@@ -3000,32 +3018,32 @@ BYTE* emitter::emitOutputInstr_OptsRlNoReloc(BYTE* dst, ssize_t igOffs, regNumbe
30003018

30013019
BYTE* emitter::emitOutputInstr_OptsJalr(BYTE* dst, instrDescJmp* jmp, const insGroup* ig, instruction* ins)
30023020
{
3003-
ssize_t immediate = emitOutputInstrJumpDistance(dst, ig, jmp) - 4;
3021+
const ssize_t immediate = emitOutputInstrJumpDistance(dst, ig, jmp) - 4;
30043022
assert((immediate & 0x03) == 0);
30053023

30063024
*ins = jmp->idIns();
3007-
assert(jmp->idCodeSize() > 4); // The original INS_OPTS_JALR: not used by now!!!
30083025
switch (jmp->idCodeSize())
30093026
{
30103027
case 8:
3011-
return emitOutputInstr_OptsJalr8(dst, jmp, *ins, immediate);
3028+
return emitOutputInstr_OptsJalr8(dst, jmp, immediate);
30123029
case 24:
3013-
assert((*ins == INS_jal) || (*ins == INS_j));
3030+
assert(jmp->idInsIs(INS_jal, INS_j));
30143031
return emitOutputInstr_OptsJalr24(dst, immediate);
30153032
case 28:
3016-
return emitOutputInstr_OptsJalr28(dst, jmp, *ins, immediate);
3033+
return emitOutputInstr_OptsJalr28(dst, jmp, immediate);
30173034
default:
3035+
// case 0 - 4: The original INS_OPTS_JALR: not used by now!!!
30183036
break;
30193037
}
30203038
unreached();
30213039
return nullptr;
30223040
}
30233041

3024-
BYTE* emitter::emitOutputInstr_OptsJalr8(BYTE* dst, const instrDescJmp* jmp, instruction ins, ssize_t immediate)
3042+
BYTE* emitter::emitOutputInstr_OptsJalr8(BYTE* dst, const instrDescJmp* jmp, ssize_t immediate)
30253043
{
3026-
regNumber reg2 = ((ins != INS_beqz) && (ins != INS_bnez)) ? jmp->idReg2() : REG_R0;
3044+
const regNumber reg2 = jmp->idInsIs(INS_beqz, INS_bnez) ? REG_R0 : jmp->idReg2();
30273045

3028-
dst += emitOutput_BTypeInstr_InvertComparation(dst, ins, jmp->idReg1(), reg2, 0x8);
3046+
dst += emitOutput_BTypeInstr_InvertComparation(dst, jmp->idIns(), jmp->idReg1(), reg2, 0x8);
30293047
dst += emitOutput_JTypeInstr(dst, INS_jal, REG_ZERO, TrimSignedToImm21(immediate));
30303048
return dst;
30313049
}
@@ -3034,14 +3052,14 @@ BYTE* emitter::emitOutputInstr_OptsJalr24(BYTE* dst, ssize_t immediate)
30343052
{
30353053
// Make target address with offset, then jump (JALR) with the target address
30363054
immediate -= 2 * 4;
3037-
ssize_t high = UpperWordOfDoubleWordSingleSignExtend<0>(immediate);
3055+
const ssize_t high = UpperWordOfDoubleWordSingleSignExtend<0>(immediate);
30383056

30393057
dst += emitOutput_UTypeInstr(dst, INS_lui, REG_RA, UpperNBitsOfWordSignExtend<20>(high));
30403058
dst += emitOutput_ITypeInstr(dst, INS_addi, REG_RA, REG_RA, LowerNBitsOfWord<12>(high));
30413059
dst += emitOutput_ITypeInstr(dst, INS_slli, REG_RA, REG_RA, 32);
30423060

3043-
regNumber rsvdReg = codeGen->rsGetRsvdReg();
3044-
ssize_t low = LowerWordOfDoubleWord(immediate);
3061+
const regNumber rsvdReg = codeGen->rsGetRsvdReg();
3062+
const ssize_t low = LowerWordOfDoubleWord(immediate);
30453063

30463064
dst += emitOutput_UTypeInstr(dst, INS_auipc, rsvdReg, UpperNBitsOfWordSignExtend<20>(low));
30473065
dst += emitOutput_RTypeInstr(dst, INS_add, rsvdReg, REG_RA, rsvdReg);
@@ -3050,17 +3068,18 @@ BYTE* emitter::emitOutputInstr_OptsJalr24(BYTE* dst, ssize_t immediate)
30503068
return dst;
30513069
}
30523070

3053-
BYTE* emitter::emitOutputInstr_OptsJalr28(BYTE* dst, const instrDescJmp* jmp, instruction ins, ssize_t immediate)
3071+
BYTE* emitter::emitOutputInstr_OptsJalr28(BYTE* dst, const instrDescJmp* jmp, ssize_t immediate)
30543072
{
3055-
regNumber reg2 = ((ins != INS_beqz) && (ins != INS_bnez)) ? jmp->idReg2() : REG_R0;
3056-
dst += emitOutput_BTypeInstr_InvertComparation(dst, ins, jmp->idReg1(), reg2, 0x1c);
3073+
regNumber reg2 = jmp->idInsIs(INS_beqz, INS_bnez) ? REG_R0 : jmp->idReg2();
3074+
3075+
dst += emitOutput_BTypeInstr_InvertComparation(dst, jmp->idIns(), jmp->idReg1(), reg2, 0x1c);
30573076

30583077
return emitOutputInstr_OptsJalr24(dst, immediate);
30593078
}
30603079

30613080
BYTE* emitter::emitOutputInstr_OptsJCond(BYTE* dst, instrDesc* id, const insGroup* ig, instruction* ins)
30623081
{
3063-
ssize_t immediate = emitOutputInstrJumpDistance(dst, ig, static_cast<instrDescJmp*>(id));
3082+
const ssize_t immediate = emitOutputInstrJumpDistance(dst, ig, static_cast<instrDescJmp*>(id));
30643083

30653084
*ins = id->idIns();
30663085

@@ -3070,7 +3089,7 @@ BYTE* emitter::emitOutputInstr_OptsJCond(BYTE* dst, instrDesc* id, const insGrou
30703089

30713090
BYTE* emitter::emitOutputInstr_OptsJ(BYTE* dst, instrDesc* id, const insGroup* ig, instruction* ins)
30723091
{
3073-
ssize_t immediate = emitOutputInstrJumpDistance(dst, ig, static_cast<instrDescJmp*>(id));
3092+
const ssize_t immediate = emitOutputInstrJumpDistance(dst, ig, static_cast<instrDescJmp*>(id));
30743093
assert((immediate & 0x03) == 0);
30753094

30763095
*ins = id->idIns();
@@ -3133,11 +3152,13 @@ BYTE* emitter::emitOutputInstr_OptsC(BYTE* dst, instrDesc* id, const insGroup* i
31333152
size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
31343153
{
31353154
BYTE* dst = *dp;
3155+
BYTE* dst2 = dst + 4;
31363156
const BYTE* const odst = *dp;
31373157
instruction ins;
31383158
size_t sz = 0;
31393159

31403160
assert(REG_NA == static_cast<int>(REG_NA));
3161+
assert(writeableOffset == 0);
31413162

31423163
insOpts insOp = id->idInsOpt();
31433164

@@ -3174,8 +3195,9 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
31743195
sz = sizeof(instrDescJmp);
31753196
break;
31763197
case INS_OPTS_C:
3177-
dst = emitOutputInstr_OptsC(dst, id, ig, &sz);
3178-
ins = INS_nop;
3198+
dst = emitOutputInstr_OptsC(dst, id, ig, &sz);
3199+
dst2 = dst;
3200+
ins = INS_nop;
31793201
break;
31803202
default: // case INS_OPTS_NONE:
31813203
dst += emitOutput_Instr(dst, id->idAddr()->iiaGetInstrEncode());
@@ -3193,11 +3215,11 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
31933215
// We assume that "idReg1" is the primary destination register for all instructions
31943216
if (id->idGCref() != GCT_NONE)
31953217
{
3196-
emitGCregLiveUpd(id->idGCref(), id->idReg1(), dst);
3218+
emitGCregLiveUpd(id->idGCref(), id->idReg1(), dst2);
31973219
}
31983220
else
31993221
{
3200-
emitGCregDeadUpd(id->idReg1(), dst);
3222+
emitGCregDeadUpd(id->idReg1(), dst2);
32013223
}
32023224
}
32033225

@@ -3211,7 +3233,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
32113233
int adr = emitComp->lvaFrameAddress(varNum, &FPbased);
32123234
if (id->idGCref() != GCT_NONE)
32133235
{
3214-
emitGCvarLiveUpd(adr + ofs, varNum, id->idGCref(), dst DEBUG_ARG(varNum));
3236+
emitGCvarLiveUpd(adr + ofs, varNum, id->idGCref(), dst2 DEBUG_ARG(varNum));
32153237
}
32163238
else
32173239
{
@@ -3228,7 +3250,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
32283250
vt = tmpDsc->tdTempType();
32293251
}
32303252
if (vt == TYP_REF || vt == TYP_BYREF)
3231-
emitGCvarDeadUpd(adr + ofs, dst DEBUG_ARG(varNum));
3253+
emitGCvarDeadUpd(adr + ofs, dst2 DEBUG_ARG(varNum));
32323254
}
32333255
// if (emitInsWritesToLclVarStackLocPair(id))
32343256
//{

0 commit comments

Comments
 (0)