From 7754e37ed00967e0c11d034b55bc2b1d77403846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 23 May 2025 16:45:39 +0200 Subject: [PATCH 1/2] Reverse unordered comparisons --- src/coreclr/jit/lowerriscv64.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lowerriscv64.cpp b/src/coreclr/jit/lowerriscv64.cpp index 9f410e1e09ab4e..19d069acf700d7 100644 --- a/src/coreclr/jit/lowerriscv64.cpp +++ b/src/coreclr/jit/lowerriscv64.cpp @@ -136,11 +136,10 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) GenTree* cmpOp1; GenTree* cmpOp2; + assert(!op->OperIsCompare() || op->OperIsCmpCompare()); // We do not expect any other relops on RISCV64 + if (op->OperIsCompare() && !varTypeIsFloating(op->gtGetOp1())) { - // We do not expect any other relops on RISCV64 - assert(op->OperIs(GT_EQ, GT_NE, GT_LT, GT_LE, GT_GE, GT_GT)); - cond = GenCondition::FromRelop(op); cmpOp1 = op->gtGetOp1(); @@ -151,7 +150,16 @@ GenTree* Lowering::LowerJTrue(GenTreeOp* jtrue) } else { - cond = GenCondition(GenCondition::NE); + GenCondition::Code code = GenCondition::NE; + if (op->OperIsCompare() && varTypeIsFloating(op->gtGetOp1()) && (op->gtFlags & GTF_RELOP_NAN_UN) != 0) + { + // Unordered floating-point comparisons are achieved by neg'ing the ordered counterparts. Avoid that by + // reversing both the FP comparison and the zero-comparison fused with the branch. + op->ChangeOper(GenTree::ReverseRelop(op->OperGet())); + op->gtFlags &= ~GTF_RELOP_NAN_UN; + code = GenCondition::EQ; + } + cond = GenCondition(code); cmpOp1 = op; cmpOp2 = comp->gtNewZeroConNode(cmpOp1->TypeGet()); From d6095657b1b72c093baf6b57a8b6f05a47ec5f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Sowi=C5=84ski?= Date: Fri, 23 May 2025 16:45:59 +0200 Subject: [PATCH 2/2] Codegen haircut --- src/coreclr/jit/codegenriscv64.cpp | 77 ++++++++++-------------------- 1 file changed, 25 insertions(+), 52 deletions(-) diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index dd42db7aca28b6..2af0a29901a351 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -3136,65 +3136,38 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree) if (varTypeIsFloating(op1Type)) { - bool isUnordered = (tree->gtFlags & GTF_RELOP_NAN_UN) != 0; - regNumber regOp1 = op1->GetRegNum(); - regNumber regOp2 = op2->GetRegNum(); + assert(!op2->isContainedIntOrIImmed()); + assert(op1Type == op2Type); + genTreeOps oper = tree->OperGet(); + bool isUnordered = (tree->gtFlags & GTF_RELOP_NAN_UN) != 0; if (isUnordered) { - if (tree->OperIs(GT_LT)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_fle_s : INS_fle_d, cmpSize, targetReg, regOp2, regOp1); - } - else if (tree->OperIs(GT_LE)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_flt_s : INS_flt_d, cmpSize, targetReg, regOp2, regOp1); - } - else if (tree->OperIs(GT_NE)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_feq_s : INS_feq_d, cmpSize, targetReg, regOp1, regOp2); - } - else if (tree->OperIs(GT_GT)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_fle_s : INS_fle_d, cmpSize, targetReg, regOp1, regOp2); - } - else if (tree->OperIs(GT_GE)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_flt_s : INS_flt_d, cmpSize, targetReg, regOp1, regOp2); - } - else - { - unreached(); - } - emit->emitIns_R_R_I(INS_xori, EA_8BYTE, targetReg, targetReg, 1); + oper = GenTree::ReverseRelop(oper); } - else + if (oper == GT_GT || oper == GT_GE) { - if (tree->OperIs(GT_LT)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_flt_s : INS_flt_d, cmpSize, targetReg, regOp1, regOp2); - } - else if (tree->OperIs(GT_LE)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_fle_s : INS_fle_d, cmpSize, targetReg, regOp1, regOp2); - } - else if (tree->OperIs(GT_EQ)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_feq_s : INS_feq_d, cmpSize, targetReg, regOp1, regOp2); - } - else if (tree->OperIs(GT_GT)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_flt_s : INS_flt_d, cmpSize, targetReg, regOp2, regOp1); - } - else if (tree->OperIs(GT_GE)) - { - emit->emitIns_R_R_R(cmpSize == EA_4BYTE ? INS_fle_s : INS_fle_d, cmpSize, targetReg, regOp2, regOp1); - } - else - { + oper = GenTree::SwapRelop(oper); + std::swap(op1, op2); + } + instruction instr = INS_none; + switch (oper) + { + case GT_LT: + instr = (cmpSize == EA_4BYTE) ? INS_flt_s : INS_flt_d; + break; + case GT_LE: + instr = (cmpSize == EA_4BYTE) ? INS_fle_s : INS_fle_d; + break; + case GT_EQ: + instr = (cmpSize == EA_4BYTE) ? INS_feq_s : INS_feq_d; + break; + default: unreached(); - } } + emit->emitIns_R_R_R(instr, cmpSize, targetReg, op1->GetRegNum(), op2->GetRegNum()); + if (isUnordered) + emit->emitIns_R_R_I(INS_xori, EA_8BYTE, targetReg, targetReg, 1); } else {