From b8f8848866ba621f7ee2ee82a799c8c583b5977f Mon Sep 17 00:00:00 2001 From: Sanghyeon Date: Thu, 2 Oct 2025 22:00:45 +0900 Subject: [PATCH 1/5] [RISC-V] Optimize redundant sext.w generation in GT_JCMP codegen * Peephole optimization for redundant sext.w --- src/coreclr/jit/codegenriscv64.cpp | 43 +++++++++++--- src/coreclr/jit/emitriscv64.cpp | 95 ++++++++++++++++++++++++++++++ src/coreclr/jit/emitriscv64.h | 3 + 3 files changed, 132 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 6deaa25f599831..5e2d64f3b124fd 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -3188,7 +3188,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree) regOp1 = tmpRegOp1; } } - + if (tree->OperIs(GT_EQ, GT_NE)) { if ((imm != 0) || (cmpSize == EA_4BYTE)) @@ -3353,8 +3353,14 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) regNumber tmpRegOp1 = rsGetRsvdReg(); assert(regOp1 != tmpRegOp1); imm = static_cast(imm); - emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); - regOp1 = tmpRegOp1; + + // It might be possible to watch the type of op1 to decide the redundancy. + // But due to the safety reason, only a peephole optimization is done now. + if (!(emit->isRedundantSignExtend(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1))) + { + emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); + regOp1 = tmpRegOp1; + } break; } case EA_8BYTE: @@ -3389,8 +3395,14 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) { regNumber tmpRegOp1 = rsGetRsvdReg(); assert(regOp1 != tmpRegOp1); - emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); - regOp1 = tmpRegOp1; + + // It might be possible to watch the type of op1 to decide the redundancy. + // But due to the safety reason, only a peephole optimization is done now. + if (!(emit->isRedundantSignExtend(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1))) + { + emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); + regOp1 = tmpRegOp1; + } } } @@ -3438,10 +3450,23 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) regNumber tmpRegOp2 = rsGetRsvdReg(); assert(regOp1 != tmpRegOp2); assert(regOp2 != tmpRegOp2); - emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); - emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp2, regOp2); - regOp1 = tmpRegOp1; - regOp2 = tmpRegOp2; + + // It might be possible to watch the type of op1 to decide the redundancy. + // But due to the safety reason, only a peephole optimization is done now. + bool signExtOp1 = !(emit->isRedundantSignExtend(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1)); + bool signExtOp2 = !(emit->isRedundantSignExtend(INS_sext_w, EA_8BYTE, tmpRegOp2, regOp2)); + + if (signExtOp1) + { + emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); + regOp1 = tmpRegOp1; + } + + if (signExtOp2) + { + emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp2, regOp2); + regOp2 = tmpRegOp2; + } } switch (cond.GetCode()) diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index bf11222800d09b..4809c14a433481 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -640,6 +640,101 @@ void emitter::emitIns_Mov(emitAttr attr, regNumber dstReg, regNumber srcReg, boo } } +//------------------------------------------------------------------------ +// emitInsIsSignExtend: Determines whether a given instruction sign extends +// +// Arguments: +// ins -- The instruction being checked +// +bool emitter::emitInsIsSignExtend(instruction ins) +{ + switch(ins) + { + case INS_sext_w: // R_R + case INS_lui: // R_I + + // R_R_I + case INS_lb: + case INS_lh: + case INS_lw: + + // R_R_I + case INS_addiw: + case INS_slliw: + case INS_srliw: + case INS_sraiw: + + // R_R_R + case INS_addw: + case INS_subw: + case INS_sllw: + case INS_srlw: + case INS_sraw: + + // R_R_R + case INS_mulw: + case INS_divw: + case INS_divuw: + case INS_remw: + case INS_remuw: + { + return true; + } + // TODO: Add more sign-extension instructions + default: + { + return false; + } + } +} + +//------------------------------------------------------------------------ +// isRedundantSignExtend: +// Check if the current 'sext.w' instruction is redundant and can be omitted. +// +// A 'sext.w' instruction is redundant if the previous instruction sign extends +// the source register of current instruction. +// +// Arguments: +// ins -- The instruction being emitted +// attr -- The emit attribute +// dst -- The destination register +// src -- The source register +// +bool emitter::isRedundantSignExtend(instruction ins, emitAttr size, regNumber dst, regNumber src) +{ + assert(ins == INS_sext_w); + + if (!emitComp->opts.OptimizationEnabled()) + { + return false; + } + + const bool canOptimize = emitCanPeepholeLastIns(); + + if (!canOptimize) + { + return false; + } + + regNumber prevDst = emitLastIns->idReg1(); + regNumber prevSrc = emitLastIns->idReg2(); + emitAttr prevSize = emitLastIns->idOpSize(); + + bool isPrevInsSignExtend = emitInsIsSignExtend(emitLastIns->idIns()); + + if (isPrevInsSignExtend && (prevDst == src)) + { + JITDUMP("\n -- suppressing 'sext.w reg%u, reg%u' as previous instruction already sign-extended " + "the register reg%u.\n", + dst, src, prevDst); + + return true; + } + + return false; +} + /***************************************************************************** * * Add an instruction referencing two registers diff --git a/src/coreclr/jit/emitriscv64.h b/src/coreclr/jit/emitriscv64.h index 57f7472c245934..41e52c26b4383f 100644 --- a/src/coreclr/jit/emitriscv64.h +++ b/src/coreclr/jit/emitriscv64.h @@ -19,6 +19,8 @@ struct CnsVal bool cnsReloc; }; +bool isRedundantSignExtend(instruction ins, emitAttr size, regNumber reg1, regNumber reg2); + #ifdef DEBUG /************************************************************************/ @@ -61,6 +63,7 @@ instrDesc* emitNewInstrCallInd(int argCnt, bool emitInsIsLoad(instruction ins); bool emitInsIsStore(instruction ins); bool emitInsIsLoadOrStore(instruction ins); +bool emitInsIsSignExtend(instruction ins); void emitDispInsName( code_t code, const BYTE* addr, bool doffs, unsigned insOffset, const instrDesc* id, const insGroup* ig); From 0b7414186e116bd1615f5f3ab59b49c5de9db612 Mon Sep 17 00:00:00 2001 From: Sanghyeon Lee Date: Sun, 12 Oct 2025 05:47:49 +0900 Subject: [PATCH 2/5] Update src/coreclr/jit/emitriscv64.h Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/jit/emitriscv64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/emitriscv64.h b/src/coreclr/jit/emitriscv64.h index 41e52c26b4383f..b75feeed8b9fe1 100644 --- a/src/coreclr/jit/emitriscv64.h +++ b/src/coreclr/jit/emitriscv64.h @@ -19,7 +19,7 @@ struct CnsVal bool cnsReloc; }; -bool isRedundantSignExtend(instruction ins, emitAttr size, regNumber reg1, regNumber reg2); +bool isRedundantSignExtend(instruction ins, emitAttr size, regNumber dst, regNumber src); #ifdef DEBUG From 172f121970cf2a20aba6b65d40d7a4031b6eabc9 Mon Sep 17 00:00:00 2001 From: Sanghyeon Lee Date: Sun, 12 Oct 2025 05:48:04 +0900 Subject: [PATCH 3/5] Update src/coreclr/jit/emitriscv64.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- 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 4809c14a433481..d5eac22dd6180e 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -648,7 +648,7 @@ void emitter::emitIns_Mov(emitAttr attr, regNumber dstReg, regNumber srcReg, boo // bool emitter::emitInsIsSignExtend(instruction ins) { - switch(ins) + switch (ins) { case INS_sext_w: // R_R case INS_lui: // R_I From 891a072938488cf0cdea65c8b9b5437a90e816eb Mon Sep 17 00:00:00 2001 From: Sanghyeon Lee Date: Sun, 12 Oct 2025 05:49:11 +0900 Subject: [PATCH 4/5] Update src/coreclr/jit/codegenriscv64.cpp Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com> --- src/coreclr/jit/codegenriscv64.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 5e2d64f3b124fd..c38d629940fd4f 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -3188,7 +3188,6 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree) regOp1 = tmpRegOp1; } } - if (tree->OperIs(GT_EQ, GT_NE)) { if ((imm != 0) || (cmpSize == EA_4BYTE)) From 22960e2ce3e39cd0b98f3eaf9bfd710ee364d69a Mon Sep 17 00:00:00 2001 From: Sanghyeon Date: Mon, 13 Oct 2025 15:46:49 +0900 Subject: [PATCH 5/5] Fix jit-format --- src/coreclr/jit/codegenriscv64.cpp | 4 ++-- src/coreclr/jit/emitriscv64.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index c38d629940fd4f..021343e95173b3 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -3454,13 +3454,13 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) // But due to the safety reason, only a peephole optimization is done now. bool signExtOp1 = !(emit->isRedundantSignExtend(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1)); bool signExtOp2 = !(emit->isRedundantSignExtend(INS_sext_w, EA_8BYTE, tmpRegOp2, regOp2)); - + if (signExtOp1) { emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp1, regOp1); regOp1 = tmpRegOp1; } - + if (signExtOp2) { emit->emitIns_R_R(INS_sext_w, EA_8BYTE, tmpRegOp2, regOp2); diff --git a/src/coreclr/jit/emitriscv64.cpp b/src/coreclr/jit/emitriscv64.cpp index d5eac22dd6180e..6f78573fb18635 100644 --- a/src/coreclr/jit/emitriscv64.cpp +++ b/src/coreclr/jit/emitriscv64.cpp @@ -651,7 +651,7 @@ bool emitter::emitInsIsSignExtend(instruction ins) switch (ins) { case INS_sext_w: // R_R - case INS_lui: // R_I + case INS_lui: // R_I // R_R_I case INS_lb: @@ -717,15 +717,15 @@ bool emitter::isRedundantSignExtend(instruction ins, emitAttr size, regNumber ds return false; } - regNumber prevDst = emitLastIns->idReg1(); - regNumber prevSrc = emitLastIns->idReg2(); + regNumber prevDst = emitLastIns->idReg1(); + regNumber prevSrc = emitLastIns->idReg2(); emitAttr prevSize = emitLastIns->idOpSize(); bool isPrevInsSignExtend = emitInsIsSignExtend(emitLastIns->idIns()); if (isPrevInsSignExtend && (prevDst == src)) { - JITDUMP("\n -- suppressing 'sext.w reg%u, reg%u' as previous instruction already sign-extended " + JITDUMP("\n -- suppressing 'sext.w reg%u, reg%u' as previous instruction already sign-extended " "the register reg%u.\n", dst, src, prevDst);