Skip to content

[RISC-V] De-intrinsify Min/MaxMagnitude #118589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Oct 2, 2025
Merged

[RISC-V] De-intrinsify Min/MaxMagnitude #118589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d97e2c0
De-intrinsify Min/MaxMagnitude
tomeksowi Aug 11, 2025
0d012b4
cleanup
tomeksowi Aug 11, 2025
8683382
Merge branch 'main' into minmax-magnitude-fallback
tomeksowi Sep 22, 2025
b773bce
Implement new NaN preservation requirements
tomeksowi Sep 23, 2025
e8810e6
FP min/max are no longer commutative
tomeksowi Sep 23, 2025
f7d0045
remove inner qmark
tomeksowi Sep 24, 2025
8443bee
MinNumber to MinNative
tomeksowi Sep 24, 2025
ff14c21
Fix dropped NOT node on qmark condition reversal
tomeksowi Sep 24, 2025
3ba23a4
cleanup
tomeksowi Sep 25, 2025
2838bae
use EQ(cond, 0) to reverse cond instead of NOT
tomeksowi Sep 25, 2025
6feacbd
add case for constant condition
tomeksowi Sep 25, 2025
bb73d1c
cleanup
tomeksowi Sep 25, 2025
95a7c88
fix comments
tomeksowi Oct 1, 2025
86588d8
use impCloneExpr
tomeksowi Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/coreclr/jit/codegenriscv64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
23 changes: 14 additions & 9 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -12817,17 +12822,17 @@ 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");
break;
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");
Expand Down
108 changes: 60 additions & 48 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/lsrariscv64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 4 additions & 6 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -14530,6 +14527,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
elseBlock->SetFlags(propagateFlagsToAll);

BasicBlock* thenBlock = nullptr;

if (hasTrueExpr && hasFalseExpr)
{
// bbj_always
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
28 changes: 14 additions & 14 deletions src/coreclr/jit/valuenum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9861,15 +9861,15 @@ 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);
res = FloatingPointUtils::maximumNumber(arg0Val, arg1Val);
break;
}

case NI_System_Math_MinNumber:
case NI_System_Math_MinNative:
{
assert(typ == TypeOfVN(arg1VN));
double arg1Val = GetConstantDouble(arg1VN);
Expand Down Expand Up @@ -9903,15 +9903,15 @@ 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);
res = FloatingPointUtils::maximumNumber(arg0Val, arg1Val);
break;
}

case NI_System_Math_MinNumber:
case NI_System_Math_MinNative:
{
assert(typ == TypeOfVN(arg1VN));
float arg1Val = GetConstantSingle(arg1VN);
Expand Down Expand Up @@ -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

Expand Down
23 changes: 13 additions & 10 deletions src/coreclr/jit/valuenumfuncs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Loading