Skip to content

JIT: Cleanup and harden lowering #130837

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
tannergooding merged 4 commits into from
Jul 21, 2026
Merged

JIT: Cleanup and harden lowering #130837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c7316d
Fix debug-only diagnostics in lower.cpp arg splitting
tannergooding Jul 16, 2026
17aa565
Harden ARM64 shift/NOT containment and drop dead stores
tannergooding Jul 16, 2026
df815f0
Remove dead code in lowerxarch.cpp lowering
tannergooding Jul 16, 2026
8752867
Correct comments and typos in lowerxarch.cpp
tannergooding Jul 16, 2026
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
14 changes: 7 additions & 7 deletions src/coreclr/jit/lower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void Lowering::MakeSrcContained(GenTree* parentNode, GenTree* childNode) const

if (!isSafeToContainMem)
{
JITDUMP("** Unsafe mem containment of [%06u] in [%06u}\n", m_compiler->dspTreeID(childNode),
JITDUMP("** Unsafe mem containment of [%06u] in [%06u]\n", m_compiler->dspTreeID(childNode),
m_compiler->dspTreeID(parentNode));
assert(isSafeToContainMem);
}
Expand Down Expand Up @@ -82,7 +82,7 @@ void Lowering::MakeSrcRegOptional(GenTree* parentNode, GenTree* childNode) const

if (!isSafeToMarkRegOptional)
{
JITDUMP("** Unsafe regOptional of [%06u] in [%06u}\n", m_compiler->dspTreeID(childNode),
JITDUMP("** Unsafe regOptional of [%06u] in [%06u]\n", m_compiler->dspTreeID(childNode),
m_compiler->dspTreeID(parentNode));
assert(isSafeToMarkRegOptional);
}
Expand Down Expand Up @@ -1717,7 +1717,7 @@ void Lowering::SplitArgumentBetweenRegistersAndStack(GenTreeCall* call, CallArg*

if (arg->OperIsFieldList())
{
JITDUMP("Argument is a FIELD_LIST\n", numRegs, stackSeg.Size);
JITDUMP("Argument is a FIELD_LIST\n");

GenTreeFieldList::Use* splitPoint = nullptr;
// Split the field list into its register and stack parts.
Expand All @@ -1740,7 +1740,7 @@ void Lowering::SplitArgumentBetweenRegistersAndStack(GenTreeCall* call, CallArg*

if (splitPoint == nullptr)
{
JITDUMP("No clean split point found, spilling FIELD_LIST\n", splitPoint->GetOffset());
JITDUMP("No clean split point found, spilling FIELD_LIST\n");

unsigned int newLcl =
StoreFieldListToNewLocal(m_compiler->typGetObjLayout(callArg->GetSignatureClassHandle()),
Expand Down Expand Up @@ -1779,7 +1779,7 @@ void Lowering::SplitArgumentBetweenRegistersAndStack(GenTreeCall* call, CallArg*
}
else if (arg->OperIs(GT_BLK))
{
JITDUMP("Argument is a BLK\n", numRegs, stackSeg.Size);
JITDUMP("Argument is a BLK\n");

GenTree* blkAddr = arg->AsBlk()->Addr();
target_ssize_t offset = 0;
Expand All @@ -1794,12 +1794,12 @@ void Lowering::SplitArgumentBetweenRegistersAndStack(GenTreeCall* call, CallArg*
!m_compiler->lvaGetDesc(addrUse.Def()->AsLclVarCommon())->IsAddressExposed() &&
IsInvariantInRange(addrUse.Def(), arg))
{
JITDUMP("Reusing LCL_VAR\n", numRegs, stackSeg.Size);
JITDUMP("Reusing LCL_VAR\n");
addrLcl = addrUse.Def()->AsLclVarCommon()->GetLclNum();
}
else
{
JITDUMP("Spilling address\n", numRegs, stackSeg.Size);
JITDUMP("Spilling address\n");
addrLcl = addrUse.ReplaceWithLclVar(m_compiler);
}

Expand Down
11 changes: 6 additions & 5 deletions src/coreclr/jit/lowerarmarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,13 @@ bool Lowering::IsContainableUnaryOrBinaryOp(GenTree* parentNode, GenTree* childN
}
}

if (childNode->OperIs(GT_LSH, GT_RSH, GT_RSZ) && parentNode->OperIs(GT_NOT, GT_AND_NOT, GT_OR_NOT, GT_XOR_NOT))
if (parentNode->OperIs(GT_NOT, GT_AND_NOT, GT_OR_NOT, GT_XOR_NOT))
{
return true;
if (IsInvariantInRange(childNode, parentNode))
{
assert(shiftAmountNode->isContained());
return true;
}
}

// TODO: Handle CMN, NEG/NEGS, BIC/BICS, EON, MVN, ORN, TST
Expand Down Expand Up @@ -1510,18 +1514,15 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
{
if (oper == GT_AND)
{
oper = GT_AND_NOT;
intrinsicId = NI_AdvSimd_BitwiseClear;
}
else
{
assert(oper == GT_OR);
oper = GT_NONE;
intrinsicId = NI_AdvSimd_OrNot;
}

node->ChangeHWIntrinsicId(intrinsicId, op1, op2);
oper = GT_AND_NOT;
}
break;
}
Expand Down
28 changes: 13 additions & 15 deletions src/coreclr/jit/lowerxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,6 @@ void Lowering::LowerPutArgStk(GenTreePutArgStk* putArgStk)
return;
}

assert(!src->TypeIs(TYP_STRUCT));

// If the child of GT_PUTARG_STK is a constant, we don't need a register to
// move it to memory (stack location).
//
Expand Down Expand Up @@ -662,7 +660,7 @@ void Lowering::LowerCast(GenTree* tree)
//
// This creates the equivalent of the following C# code:
// var addRes = Sse2.AddScalar(castResult, Vector128.CreateScalar(4294967296.0));
// castResult = Sse41.BlendVariable(castResult, addRes, castResult);
// castResult = X86Base.BlendVariable(castResult, addRes, castResult);
Comment thread
tannergooding marked this conversation as resolved.

GenTreeVecCon* addCns = m_compiler->gtNewVconNode(TYP_SIMD16);
addCns->gtSimdVal.f64[0] = 4294967296.0;
Expand Down Expand Up @@ -985,7 +983,7 @@ void Lowering::LowerCast(GenTree* tree)
// this is adequate to force selection of the negated result.
//
// This creates the equivalent of the following C# code:
// convertResult = Sse41.BlendVariable(result, negated, result);
// convertResult = X86Base.BlendVariable(result, negated, result);

convertResult =
m_compiler->gtNewSimdHWIntrinsicNode(TYP_SIMD16, result, negated, resultClone,
Expand Down Expand Up @@ -1458,7 +1456,7 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
GenTreeHWIntrinsic* userIntrin = user->AsHWIntrinsic();

bool userIsScalar = false;
genTreeOps userOper = userIntrin->GetOperForHWIntrinsicId(&isScalar);
genTreeOps userOper = userIntrin->GetOperForHWIntrinsicId(&userIsScalar);

// userIntrin may have re-interpreted the base type
//
Expand Down Expand Up @@ -1549,7 +1547,7 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
// B: op1
// C: op2 (AllBitsSet)
//
// This represents a double not, so so just return op2
// This represents a double not, so just return op2
// which is the only actual value now that the parameters
// were shifted around

Expand Down Expand Up @@ -1892,7 +1890,6 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
testIntrinsicId = NI_AVX512_PTESTM;
}

node->Op(1) = op1;
BlockRange().Remove(op2);

LIR::Use op1Use(BlockRange(), &node->Op(1), node);
Expand Down Expand Up @@ -2191,7 +2188,7 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
}
else
{
// We're an unused zero constant node, so don't both creating
// We're an unused zero constant node, so don't bother creating
// a new node for something that will never be consumed
}

Expand Down Expand Up @@ -2427,7 +2424,8 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node)
}
assert(varTypeIsIntegral(node->GetSimdBaseType()));

// pre-AVX512 doesn't actually support these intrinsics in hardware so we need to swap the operands around
// There's no integer compare-less-than instruction, so the managed intrinsic ID is unconditionally
// rewritten to compare-greater-than with the operands swapped
NamedIntrinsic newIntrinsicId = NI_Illegal;

switch (intrinsicId)
Expand Down Expand Up @@ -5816,7 +5814,7 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)
case TYP_DOUBLE:
{
// We will be constructing the following parts:
// idx = CNS_INT int 0x31
// idx = CNS_INT int 0x33
// /--* op1 simd16
// +--* op2 simd16
// +--* idx int
Expand All @@ -5825,7 +5823,7 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)
// node = * HWINTRINSIC simd16 T ToScalar

// This is roughly the following managed code:
// var tmp3 = Avx.DotProduct(op1, op2, 0x31);
// var tmp3 = Avx.DotProduct(op1, op2, 0x33);
// return tmp3.ToScalar();

idx = m_compiler->gtNewIconNode(0x33, TYP_INT);
Expand Down Expand Up @@ -6206,7 +6204,7 @@ GenTree* Lowering::LowerHWIntrinsicToScalar(GenTreeHWIntrinsic* node)
uint32_t elemSize = genTypeSize(simdBaseType);

GenTreeLclVarCommon* lclVar = op1->AsLclVarCommon();
uint32_t lclOffs = lclVar->GetLclOffs() + (0 * elemSize);
uint32_t lclOffs = lclVar->GetLclOffs();
LclVarDsc* lclDsc = m_compiler->lvaGetDesc(lclVar);

if (lclDsc->lvDoNotEnregister && (lclOffs <= 0xFFFF) && ((lclOffs + elemSize) <= lclDsc->lvExactSize()))
Expand Down Expand Up @@ -6718,7 +6716,7 @@ bool Lowering::IsRMWIndirCandidate(GenTree* operand, GenTree* storeInd)

if (m_scratchSideEffects.InterferesWith(m_compiler, node, false))
{
// The indirection's tree contains some node that can't be moved to the storeInder. The indirection is
// The indirection's tree contains some node that can't be moved to the storeIndir. The indirection is
// not a candidate. Clear any leftover mark bits and return.
for (; markCount > 0; node = node->gtPrev)
{
Expand Down Expand Up @@ -6824,7 +6822,7 @@ bool Lowering::IsBinOpInRMWStoreInd(GenTree* tree)
// Parameters:
// tree - GT_STOREIND node
// outIndirCandidate - out param set to indirCandidate as described above
// ouutIndirOpSource - out param set to indirOpSource as described above
// outIndirOpSource - out param set to indirOpSource as described above
//
// Return value
// True if there is a RMW memory operation rooted at a GT_STOREIND tree
Expand Down Expand Up @@ -7621,7 +7619,7 @@ void Lowering::ContainCheckMul(GenTreeOp* node)
else if (node->OperIs(GT_MUL_LONG))
{
hasImpliedFirstOperand = true;
// GT_MUL_LONG hsa node type LONG but work on INT
// GT_MUL_LONG has node type LONG but work on INT
nodeType = TYP_INT;
}
#endif
Expand Down
Loading