diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 943cd62e137b30..e581bc44d7029b 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -4563,10 +4563,10 @@ void CodeGen::genIntrinsic(GenTreeIntrinsic* treeNode) case NI_System_Math_Sqrt: instr = is4 ? INS_fsqrt_s : INS_fsqrt_d; break; - case NI_System_Math_MinNumber: + case NI_System_Math_MinNative: instr = is4 ? INS_fmin_s : INS_fmin_d; break; - case NI_System_Math_MaxNumber: + case NI_System_Math_MaxNative: instr = is4 ? INS_fmax_s : INS_fmax_d; break; case NI_System_Math_Min: diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9e940bf3f01785..dbdb7c7485108d 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -3696,9 +3696,14 @@ GenTree* Compiler::gtReverseCond(GenTree* tree) GenTreeOpCC* opCC = tree->AsOpCC(); opCC->gtCondition = GenCondition::Reverse(opCC->gtCondition); } + else if (tree->IsIntegralConst()) + { + GenTreeIntConCommon* con = tree->AsIntConCommon(); + con->SetIntegralValue(con->IsIntegralConst(0) ? 1 : 0); + } else { - tree = gtNewOperNode(GT_NOT, TYP_INT, tree); + tree = gtNewOperNode(GT_EQ, TYP_INT, tree, gtNewZeroConNode(TYP_INT)); } return tree; @@ -5462,11 +5467,11 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) case NI_System_Math_Log10: #if defined(TARGET_RISCV64) case NI_System_Math_Max: - case NI_System_Math_MaxNumber: case NI_System_Math_MaxUnsigned: + case NI_System_Math_MaxNative: case NI_System_Math_Min: - case NI_System_Math_MinNumber: case NI_System_Math_MinUnsigned: + case NI_System_Math_MinNative: #endif // TARGET_RISCV64 case NI_System_Math_Pow: case NI_System_Math_Round: @@ -5817,11 +5822,11 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) #if defined(TARGET_RISCV64) case NI_System_Math_Max: - case NI_System_Math_MaxNumber: case NI_System_Math_MaxUnsigned: + case NI_System_Math_MaxNative: case NI_System_Math_Min: - case NI_System_Math_MinNumber: case NI_System_Math_MinUnsigned: + case NI_System_Math_MinNative: { level++; break; @@ -12817,8 +12822,8 @@ void Compiler::gtDispTree(GenTree* tree, case NI_System_Math_Max: printf(" max"); break; - case NI_System_Math_MaxNumber: - printf(" maxNumber"); + case NI_System_Math_MaxNative: + printf(" maxNative"); break; case NI_System_Math_MaxUnsigned: printf(" maxUnsigned"); @@ -12826,8 +12831,8 @@ void Compiler::gtDispTree(GenTree* tree, case NI_System_Math_Min: printf(" min"); break; - case NI_System_Math_MinNumber: - printf(" minNumber"); + case NI_System_Math_MinNative: + printf(" minNative"); break; case NI_System_Math_MinUnsigned: printf(" minUnsigned"); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 60d659c18f119b..d4592b4c5b7d08 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -4973,60 +4973,74 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, #endif // FEATURE_HW_INTRINSICS #ifdef TARGET_RISCV64 - GenTree* op2 = impImplicitR4orR8Cast(impPopStack().val, callType); - GenTree* op1 = impImplicitR4orR8Cast(impPopStack().val, callType); - - if (isNative) + if (!isMagnitude) { - assert(!isMagnitude && !isNumber); - isNumber = true; - } + GenTree* op2 = impImplicitR4orR8Cast(impPopStack().val, callType); + GenTree* op1 = impImplicitR4orR8Cast(impPopStack().val, callType); - GenTree *op1Clone = nullptr, *op2Clone = nullptr; + GenTree *op1Clone = nullptr, *op2Clone = nullptr; + if (isNative) + { + assert(!isMagnitude && !isNumber); + } + else + { + if (!isNumber) + { + op2 = impCloneExpr(op2, &op2Clone, CHECK_SPILL_ALL, + nullptr DEBUGARG("Clone op2 for Math.Min/Max non-Number")); + } + op1 = impCloneExpr(op1, &op1Clone, CHECK_SPILL_ALL, + nullptr DEBUGARG("Clone op1 for Math.Min/Max")); + } - if (!isNumber) - { - op2 = impCloneExpr(op2, &op2Clone, CHECK_SPILL_ALL, - nullptr DEBUGARG("Clone op2 for Math.Min/Max non-Number")); - } + static const CORINFO_CONST_LOOKUP nullEntry = {IAT_VALUE}; - if (!isNumber) - { - op1 = impCloneExpr(op1, &op1Clone, CHECK_SPILL_ALL, - nullptr DEBUGARG("Clone op1 for Math.Min/Max non-Number")); - } + // Native RISC-V fmin/fmax instructions implement IEEE 754-2019 Minimum/MaximumNumber functions + // which don't specify what kind of NaN is returned if both arguments are NaN. RISC-V returns quiet + // canonical NaN in this case. + ni = isMax ? NI_System_Math_MaxNative : NI_System_Math_MinNative; + GenTree* minMax = + new (this, GT_INTRINSIC) GenTreeIntrinsic(callType, op1, op2, ni, nullptr R2RARG(nullEntry)); - static const CORINFO_CONST_LOOKUP nullEntry = {IAT_VALUE}; - if (isMagnitude) - { - op1 = new (this, GT_INTRINSIC) - GenTreeIntrinsic(callType, op1, NI_System_Math_Abs, nullptr R2RARG(nullEntry)); - op2 = new (this, GT_INTRINSIC) - GenTreeIntrinsic(callType, op2, NI_System_Math_Abs, nullptr R2RARG(nullEntry)); - } + if (!isNative) + { + // Make sure we return the NaN argument verbatim (if both are NaN, the first one), which is an + // additional requirement for .NET Min/Max APIs on top of IEEE 754. - ni = isMax ? NI_System_Math_MaxNumber : NI_System_Math_MinNumber; - GenTree* minMax = - new (this, GT_INTRINSIC) GenTreeIntrinsic(callType, op1, op2, ni, nullptr R2RARG(nullEntry)); + if (isNumber) + { + // Build expression: isNumber(minMax) ? minMax : op1 + GenTree* minMaxClone; + minMax = impCloneExpr(minMax, &minMaxClone, CHECK_SPILL_NONE, + nullptr DEBUGARG("Clone min/max result in Math.Min/MaxNumber")); - if (!isNumber) - { - GenTreeOp* isOp1Number = gtNewOperNode(GT_EQ, TYP_INT, op1Clone, gtCloneExpr(op1Clone)); - GenTreeOp* isOp2Number = gtNewOperNode(GT_EQ, TYP_INT, op2Clone, gtCloneExpr(op2Clone)); - GenTreeOp* isOkForMinMax = gtNewOperNode(GT_EQ, TYP_INT, isOp1Number, isOp2Number); + GenTreeOp* isNumber = gtNewOperNode(GT_EQ, TYP_INT, minMax, minMaxClone); - GenTreeOp* nanPropagator = - gtNewOperNode(GT_ADD, callType, gtCloneExpr(op1Clone), gtCloneExpr(op2Clone)); + minMax = gtNewQmarkNode(callType, isNumber, + gtNewColonNode(callType, gtCloneExpr(minMaxClone), op1Clone)); + } + else + { + // Build expression: isNumber(op1) ? (isNumber(op2) ? minMax : op2) : op1 + GenTreeOp* isOp1Number = gtNewOperNode(GT_EQ, TYP_INT, op1Clone, gtCloneExpr(op1Clone)); + GenTreeOp* isOp2Number = gtNewOperNode(GT_EQ, TYP_INT, op2Clone, gtCloneExpr(op2Clone)); + + minMax = gtNewQmarkNode(callType, isOp2Number, + gtNewColonNode(callType, minMax, gtCloneExpr(op2Clone))); + minMax = gtNewQmarkNode(callType, isOp1Number, + gtNewColonNode(callType, minMax, gtCloneExpr(op1Clone))); + } - GenTreeQmark* qmark = - gtNewQmarkNode(callType, isOkForMinMax, gtNewColonNode(callType, minMax, nanPropagator)); - // QMARK has to be a root node - unsigned tmp = lvaGrabTemp(true DEBUGARG("Temp for Qmark in Math.Min/Max non-Number")); - impStoreToTemp(tmp, qmark, CHECK_SPILL_NONE); - minMax = gtNewLclvNode(tmp, callType); - } + // Top-level QMARK needs to be in a variable + assert(minMax->OperIs(GT_QMARK)); + unsigned tmpTop = lvaGrabTemp(true DEBUGARG("Temp for top qmark in Math.Min/Max")); + impStoreToTemp(tmpTop, minMax, CHECK_SPILL_NONE); + minMax = gtNewLclvNode(tmpTop, callType); + } - retNode = minMax; + retNode = minMax; + } #endif // TARGET_RISCV64 } @@ -8377,13 +8391,11 @@ bool Compiler::IsTargetIntrinsic(NamedIntrinsic intrinsicName) case NI_System_Math_Abs: case NI_System_Math_Sqrt: case NI_System_Math_Max: - case NI_System_Math_MaxMagnitude: - case NI_System_Math_MaxMagnitudeNumber: case NI_System_Math_MaxNumber: + case NI_System_Math_MaxNative: case NI_System_Math_Min: - case NI_System_Math_MinMagnitude: - case NI_System_Math_MinMagnitudeNumber: case NI_System_Math_MinNumber: + case NI_System_Math_MinNative: case NI_System_Math_MultiplyAddEstimate: case NI_System_Math_ReciprocalEstimate: case NI_System_Math_ReciprocalSqrtEstimate: diff --git a/src/coreclr/jit/lsrariscv64.cpp b/src/coreclr/jit/lsrariscv64.cpp index a2a6152309f121..030e4633431a28 100644 --- a/src/coreclr/jit/lsrariscv64.cpp +++ b/src/coreclr/jit/lsrariscv64.cpp @@ -360,8 +360,8 @@ int LinearScan::BuildNode(GenTree* tree) switch (tree->AsIntrinsic()->gtIntrinsicName) { // Both operands and its result must be of the same floating-point type. - case NI_System_Math_MinNumber: - case NI_System_Math_MaxNumber: + case NI_System_Math_MinNative: + case NI_System_Math_MaxNative: assert(op2 != nullptr); assert(op2->TypeIs(tree->TypeGet())); FALLTHROUGH; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index a4f09b64f6213a..83711491f23ce0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14306,10 +14306,7 @@ GenTree* Compiler::fgInitThisClass() //------------------------------------------------------------------------ // fgPreExpandQmarkChecks: Verify that the importer has created GT_QMARK nodes -// in a way we can process them. The following -// -// Returns: -// Suitable phase status. +// in a way we can process them. // // Remarks: // The following is allowed: @@ -14530,6 +14527,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt) elseBlock->SetFlags(propagateFlagsToAll); BasicBlock* thenBlock = nullptr; + if (hasTrueExpr && hasFalseExpr) { // bbj_always @@ -14541,7 +14539,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt) // bbj_cond(true) // // TODO: Remove unnecessary condition reversal - gtReverseCond(condExpr); + qmark->gtOp1 = condExpr = gtReverseCond(condExpr); thenBlock = fgNewBBafter(BBJ_ALWAYS, condBlock, true); thenBlock->SetFlags(propagateFlagsToAll); @@ -14574,7 +14572,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt) // bbj_cond(true) // // TODO: Remove unnecessary condition reversal - gtReverseCond(condExpr); + qmark->gtOp1 = condExpr = gtReverseCond(condExpr); const unsigned thenLikelihood = qmark->ThenNodeLikelihood(); const unsigned elseLikelihood = qmark->ElseNodeLikelihood(); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 7ced1db89ccb05..e2de675a651578 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9861,7 +9861,7 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF } #ifdef TARGET_RISCV64 - case NI_System_Math_MaxNumber: + case NI_System_Math_MaxNative: { assert(typ == TypeOfVN(arg1VN)); double arg1Val = GetConstantDouble(arg1VN); @@ -9869,7 +9869,7 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF break; } - case NI_System_Math_MinNumber: + case NI_System_Math_MinNative: { assert(typ == TypeOfVN(arg1VN)); double arg1Val = GetConstantDouble(arg1VN); @@ -9903,7 +9903,7 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF } #ifdef TARGET_RISCV64 - case NI_System_Math_MaxNumber: + case NI_System_Math_MaxNative: { assert(typ == TypeOfVN(arg1VN)); float arg1Val = GetConstantSingle(arg1VN); @@ -9911,7 +9911,7 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF break; } - case NI_System_Math_MinNumber: + case NI_System_Math_MinNative: { assert(typ == TypeOfVN(arg1VN)); float arg1Val = GetConstantSingle(arg1VN); @@ -9987,27 +9987,27 @@ ValueNum ValueNumStore::EvalMathFuncBinary(var_types typ, NamedIntrinsic gtMathF #ifdef TARGET_RISCV64 case NI_System_Math_Max: - vnf = VNF_Max; + vnf = VNF_MaxInt; break; - case NI_System_Math_MaxNumber: - vnf = VNF_MaxNumber; + case NI_System_Math_MaxUnsigned: + vnf = VNF_MaxInt_UN; break; - case NI_System_Math_MaxUnsigned: - vnf = VNF_Max_UN; + case NI_System_Math_MaxNative: + vnf = VNF_MaxNumber; break; case NI_System_Math_Min: - vnf = VNF_Min; + vnf = VNF_MinInt; break; - case NI_System_Math_MinNumber: - vnf = VNF_MinNumber; + case NI_System_Math_MinUnsigned: + vnf = VNF_MinInt_UN; break; - case NI_System_Math_MinUnsigned: - vnf = VNF_Min_UN; + case NI_System_Math_MinNative: + vnf = VNF_MinNumber; break; #endif // TARGET_RISCV64 diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index 329004640aec1d..f4da699bed46c7 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -90,14 +90,14 @@ ValueNumFuncDef(ILogB, 1, false, false, false) ValueNumFuncDef(Log, 1, false, false, false) ValueNumFuncDef(Log2, 1, false, false, false) ValueNumFuncDef(Log10, 1, false, false, false) -ValueNumFuncDef(Max, 2, true, false, false) -ValueNumFuncDef(MaxMagnitude, 2, true, false, false) -ValueNumFuncDef(MaxMagnitudeNumber, 2, true, false, false) -ValueNumFuncDef(MaxNumber, 2, true, false, false) -ValueNumFuncDef(Min, 2, true, false, false) -ValueNumFuncDef(MinMagnitude, 2, true, false, false) -ValueNumFuncDef(MinMagnitudeNumber, 2, true, false, false) -ValueNumFuncDef(MinNumber, 2, true, false, false) +ValueNumFuncDef(Max, 2, false, false, false) +ValueNumFuncDef(MaxMagnitude, 2, false, false, false) +ValueNumFuncDef(MaxMagnitudeNumber, 2, false, false, false) +ValueNumFuncDef(MaxNumber, 2, false, false, false) +ValueNumFuncDef(Min, 2, false, false, false) +ValueNumFuncDef(MinMagnitude, 2, false, false, false) +ValueNumFuncDef(MinMagnitudeNumber, 2, false, false, false) +ValueNumFuncDef(MinNumber, 2, false, false, false) ValueNumFuncDef(Pow, 2, false, false, false) ValueNumFuncDef(RoundDouble, 1, false, false, false) ValueNumFuncDef(RoundInt32, 1, false, false, false) @@ -207,8 +207,11 @@ ValueNumFuncDef(HWI_##isa##_##name, ((argCount == -1) ? -1 : (argCount + 1)), (( //TODO-LOONGARCH64-CQ: add LoongArch64's Hardware Intrinsics Instructions if supported. #elif defined (TARGET_RISCV64) - ValueNumFuncDef(Min_UN, 2, true, false, false) // unsigned min/max intrinsics - ValueNumFuncDef(Max_UN, 2, true, false, false) + // Signed/Unsigned integer min/max intrinsics + ValueNumFuncDef(MinInt, 2, true, false, false) + ValueNumFuncDef(MaxInt, 2, true, false, false) + ValueNumFuncDef(MinInt_UN, 2, true, false, false) + ValueNumFuncDef(MaxInt_UN, 2, true, false, false) #else #error Unsupported platform #endif