From 2b2b4249f673c79d6af976c2153a9034080b51c7 Mon Sep 17 00:00:00 2001 From: Bajtazar Date: Fri, 22 Mar 2024 15:11:59 +0100 Subject: [PATCH 1/6] [RISC-V] Fixes --- src/coreclr/jit/emitriscv64.cpp | 44 ++++++++++++++++----------------- src/coreclr/jit/emitriscv64.h | 8 +++--- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index 9e9f9004170964..3a5816f7f1ffb7 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -693,7 +693,7 @@ void emitter::emitIns_R_R_I( code |= ((imm >> 5) & 0x3f) << 25; code |= ((imm >> 12) & 0x1) << 31; // TODO-RISCV64: Move jump logic to emitIns_J - id->idAddr()->iiaSetInstrCount(imm / sizeof(code_t)); + id->idAddr()->iiaSetInstrCount(static_cast(imm / sizeof(code_t))); } else if (ins == INS_csrrs || ins == INS_csrrw || ins == INS_csrrc) { @@ -2748,48 +2748,46 @@ ssize_t emitter::emitOutputInstrJumpDistance(const BYTE* src, const insGroup* ig return distVal; } -static constexpr size_t NBitMask(uint8_t bits) -{ - return (static_cast(1) << bits) - 1; -} +template +static constexpr size_t NBitMask = (static_cast(1) << Bits) - 1; template -static ssize_t LowerNBitsOfWord(ssize_t word) +static unsigned LowerNBitsOfWord(ssize_t word) { static_assert(MaskSize < 32, "Given mask size is bigger than the word itself"); static_assert(MaskSize > 0, "Given mask size cannot be zero"); - static constexpr size_t kMask = NBitMask(MaskSize); + static constexpr unsigned kMask = NBitMask; - return word & kMask; + return static_cast(word & kMask); } template -static ssize_t UpperNBitsOfWord(ssize_t word) +static unsigned UpperNBitsOfWord(ssize_t word) { - static constexpr size_t kShift = 32 - MaskSize; + static constexpr unsigned kShift = 32 - MaskSize; return LowerNBitsOfWord(word >> kShift); } template -static ssize_t UpperNBitsOfWordSignExtend(ssize_t word) +static unsigned UpperNBitsOfWordSignExtend(ssize_t word) { static constexpr unsigned kSignExtend = 1 << (31 - MaskSize); return UpperNBitsOfWord(word + kSignExtend); } -static ssize_t UpperWordOfDoubleWord(ssize_t immediate) +static unsigned UpperWordOfDoubleWord(ssize_t immediate) { - return immediate >> 32; + return static_cast(immediate >> 32); } -static ssize_t LowerWordOfDoubleWord(ssize_t immediate) +static unsigned LowerWordOfDoubleWord(ssize_t immediate) { - static constexpr size_t kWordMask = NBitMask(32); + static constexpr size_t kWordMask = NBitMask<32>; - return immediate & kWordMask; + return static_cast(immediate & kWordMask); } template @@ -2815,28 +2813,28 @@ static ssize_t UpperWordOfDoubleWordDoubleSignExtend(ssize_t doubleWord) return UpperWordOfDoubleWord(DoubleWordSignExtend(doubleWord)); } -/*static*/ unsigned emitter::TrimSignedToImm12(int imm12) +/*static*/ unsigned emitter::TrimSignedToImm12(ssize_t imm12) { assert(isValidSimm12(imm12)); return static_cast(LowerNBitsOfWord<12>(imm12)); } -/*static*/ unsigned emitter::TrimSignedToImm13(int imm13) +/*static*/ unsigned emitter::TrimSignedToImm13(ssize_t imm13) { assert(isValidSimm13(imm13)); return static_cast(LowerNBitsOfWord<13>(imm13)); } -/*static*/ unsigned emitter::TrimSignedToImm20(int imm20) +/*static*/ unsigned emitter::TrimSignedToImm20(ssize_t imm20) { assert(isValidSimm20(imm20)); return static_cast(LowerNBitsOfWord<20>(imm20)); } -/*static*/ unsigned emitter::TrimSignedToImm21(int imm21) +/*static*/ unsigned emitter::TrimSignedToImm21(ssize_t imm21) { assert(isValidSimm21(imm21)); @@ -2890,7 +2888,7 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im if (id->idReg2()) { // special for INT64_MAX or UINT32_MAX - dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask(12)); + dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask<12>); const ssize_t shiftValue = (immediate == INT64_MAX) ? 1 : 32; dst += emitOutput_ITypeInstr(dst, INS_srli, reg1, reg1, shiftValue); } @@ -2904,10 +2902,10 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im BYTE* emitter::emitOutputInstr_OptsI32(BYTE* dst, ssize_t immediate, regNumber reg1) { - const ssize_t upperWord = UpperWordOfDoubleWord(immediate); + const unsigned upperWord = UpperWordOfDoubleWord(immediate); dst += emitOutput_UTypeInstr(dst, INS_lui, reg1, UpperNBitsOfWordSignExtend<20>(upperWord)); dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, reg1, LowerNBitsOfWord<12>(upperWord)); - const ssize_t lowerWord = LowerWordOfDoubleWord(immediate); + const unsigned lowerWord = LowerWordOfDoubleWord(immediate); dst += emitOutput_ITypeInstr(dst, INS_slli, reg1, reg1, 11); dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, reg1, LowerNBitsOfWord<11>(lowerWord >> 21)); dst += emitOutput_ITypeInstr(dst, INS_slli, reg1, reg1, 11); diff --git a/src/coreclr/jit/emitriscv64.h b/src/coreclr/jit/emitriscv64.h index 929e2af7dd5916..aef61a029cb576 100644 --- a/src/coreclr/jit/emitriscv64.h +++ b/src/coreclr/jit/emitriscv64.h @@ -141,10 +141,10 @@ BYTE* emitOutputInstr_OptsJCond(BYTE* dst, instrDesc* id, const insGroup* ig, in BYTE* emitOutputInstr_OptsJ(BYTE* dst, instrDesc* id, const insGroup* ig, instruction* ins); BYTE* emitOutputInstr_OptsC(BYTE* dst, instrDesc* id, const insGroup* ig, size_t* size); -static unsigned TrimSignedToImm12(int imm12); -static unsigned TrimSignedToImm13(int imm13); -static unsigned TrimSignedToImm20(int imm20); -static unsigned TrimSignedToImm21(int imm21); +static unsigned TrimSignedToImm12(ssize_t imm12); +static unsigned TrimSignedToImm13(ssize_t imm13); +static unsigned TrimSignedToImm20(ssize_t imm20); +static unsigned TrimSignedToImm21(ssize_t imm21); /************************************************************************/ /* Public inline informational methods */ From abed9d94e42fb443f9fa86acad01e7dd17b65aa8 Mon Sep 17 00:00:00 2001 From: Bajtazar Date: Fri, 22 Mar 2024 15:36:29 +0100 Subject: [PATCH 2/6] [RISC-V] Final fixes --- src/coreclr/jit/emitriscv64.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index 3a5816f7f1ffb7..67957ce6daa30e 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -2889,7 +2889,7 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im { // special for INT64_MAX or UINT32_MAX dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask<12>); - const ssize_t shiftValue = (immediate == INT64_MAX) ? 1 : 32; + const unsigned shiftValue = (immediate == INT64_MAX) ? 1 : 32; dst += emitOutput_ITypeInstr(dst, INS_srli, reg1, reg1, shiftValue); } else @@ -2964,7 +2964,7 @@ BYTE* emitter::emitOutputInstr_OptsRcNoReloc(BYTE* dst, instruction* ins, unsign const regNumber rsvdReg = codeGen->rsGetRsvdReg(); const instruction lastIns = (*ins == INS_jal) ? (*ins = INS_addi) : *ins; - const UINT32 high = immediate >> 11; + const ssize_t high = immediate >> 11; dst += emitOutput_UTypeInstr(dst, INS_lui, rsvdReg, UpperNBitsOfWordSignExtend<20>(high)); dst += emitOutput_ITypeInstr(dst, INS_addi, rsvdReg, rsvdReg, LowerNBitsOfWord<12>(high)); From 61939b356f9b0d79920c8d4ee9ea557469afbea1 Mon Sep 17 00:00:00 2001 From: Bajtazar Date: Tue, 26 Mar 2024 11:15:37 +0100 Subject: [PATCH 3/6] [RISC-V] Fixed NBitMask issue --- src/coreclr/jit/emitriscv64.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index 67957ce6daa30e..f760cede7fc76c 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -2748,8 +2748,12 @@ ssize_t emitter::emitOutputInstrJumpDistance(const BYTE* src, const insGroup* ig return distVal; } -template -static constexpr size_t NBitMask = (static_cast(1) << Bits) - 1; +template +static inline constexpr typename std::conditional<(Bits > 32), size_t, unsigned>::type NBitMask() noexcept +{ + return static_cast 32), size_t, unsigned>::type>( + (static_cast(1) << Bits) - 1); +} template static unsigned LowerNBitsOfWord(ssize_t word) @@ -2757,7 +2761,7 @@ static unsigned LowerNBitsOfWord(ssize_t word) static_assert(MaskSize < 32, "Given mask size is bigger than the word itself"); static_assert(MaskSize > 0, "Given mask size cannot be zero"); - static constexpr unsigned kMask = NBitMask; + static constexpr unsigned kMask = NBitMask(); return static_cast(word & kMask); } @@ -2785,7 +2789,7 @@ static unsigned UpperWordOfDoubleWord(ssize_t immediate) static unsigned LowerWordOfDoubleWord(ssize_t immediate) { - static constexpr size_t kWordMask = NBitMask<32>; + static constexpr size_t kWordMask = NBitMask<32>(); return static_cast(immediate & kWordMask); } @@ -2888,7 +2892,7 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im if (id->idReg2()) { // special for INT64_MAX or UINT32_MAX - dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask<12>); + dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask<12>()); const unsigned shiftValue = (immediate == INT64_MAX) ? 1 : 32; dst += emitOutput_ITypeInstr(dst, INS_srli, reg1, reg1, shiftValue); } From 80c60c976e44468961cc07f78ed82c20fc1df43f Mon Sep 17 00:00:00 2001 From: Bajtazar Date: Tue, 26 Mar 2024 11:47:09 +0100 Subject: [PATCH 4/6] [RISC-V] Removed some of the redundancy in the NBitMask --- src/coreclr/jit/emitriscv64.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index f760cede7fc76c..c7f40058ec925c 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -2751,8 +2751,7 @@ ssize_t emitter::emitOutputInstrJumpDistance(const BYTE* src, const insGroup* ig template static inline constexpr typename std::conditional<(Bits > 32), size_t, unsigned>::type NBitMask() noexcept { - return static_cast 32), size_t, unsigned>::type>( - (static_cast(1) << Bits) - 1); + return static_cast())>((static_cast(1) << Bits) - 1); } template From c0b81b88e76abddea0ca417e8736315b45b46222 Mon Sep 17 00:00:00 2001 From: Bajtazar Date: Tue, 26 Mar 2024 12:05:51 +0100 Subject: [PATCH 5/6] [RISC-V] Renamed NBitMask to WordMask and simplified it's implementation --- src/coreclr/jit/emitriscv64.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index c7f40058ec925c..ad658373c8470a 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -2748,10 +2748,9 @@ ssize_t emitter::emitOutputInstrJumpDistance(const BYTE* src, const insGroup* ig return distVal; } -template -static inline constexpr typename std::conditional<(Bits > 32), size_t, unsigned>::type NBitMask() noexcept +static inline constexpr unsigned WordMask(uint8_t bits) noexcept { - return static_cast())>((static_cast(1) << Bits) - 1); + return static_cast((1ull << bits) - 1); } template @@ -2760,7 +2759,7 @@ static unsigned LowerNBitsOfWord(ssize_t word) static_assert(MaskSize < 32, "Given mask size is bigger than the word itself"); static_assert(MaskSize > 0, "Given mask size cannot be zero"); - static constexpr unsigned kMask = NBitMask(); + static constexpr unsigned kMask = WordMask(MaskSize); return static_cast(word & kMask); } @@ -2788,7 +2787,7 @@ static unsigned UpperWordOfDoubleWord(ssize_t immediate) static unsigned LowerWordOfDoubleWord(ssize_t immediate) { - static constexpr size_t kWordMask = NBitMask<32>(); + static constexpr size_t kWordMask = WordMask(32); return static_cast(immediate & kWordMask); } @@ -2891,7 +2890,7 @@ BYTE* emitter::emitOutputInstr_OptsI8(BYTE* dst, const instrDesc* id, ssize_t im if (id->idReg2()) { // special for INT64_MAX or UINT32_MAX - dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, NBitMask<12>()); + dst += emitOutput_ITypeInstr(dst, INS_addi, reg1, REG_R0, WordMask(12)); const unsigned shiftValue = (immediate == INT64_MAX) ? 1 : 32; dst += emitOutput_ITypeInstr(dst, INS_srli, reg1, reg1, shiftValue); } From b39319e705187c64bad43b0c5870fbdb5b845541 Mon Sep 17 00:00:00 2001 From: Bajtazar Date: Tue, 26 Mar 2024 12:08:54 +0100 Subject: [PATCH 6/6] [RISC-V] Removed noexcept from WordMask --- src/coreclr/jit/emitriscv64.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index ad658373c8470a..1c5be94198f8af 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -2748,7 +2748,7 @@ ssize_t emitter::emitOutputInstrJumpDistance(const BYTE* src, const insGroup* ig return distVal; } -static inline constexpr unsigned WordMask(uint8_t bits) noexcept +static inline constexpr unsigned WordMask(uint8_t bits) { return static_cast((1ull << bits) - 1); }