Skip to content

JIT: Add some more constant folding in lowering #113301

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 4 commits into from
Mar 10, 2025
Merged

JIT: Add some more constant folding in lowering #113301

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
a8a2237
JIT: Introduce `LclVarDsc::lvIsMultiRegDest`
jakobbotsch Mar 8, 2025
968be15
JIT: Add some more constant folding in lowering
jakobbotsch Mar 9, 2025
f48e9ef
Merge branch 'main' of github.com:dotnet/runtime into constant-fold-l…
jakobbotsch Mar 10, 2025
b87c458
Feedback
jakobbotsch Mar 10, 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
101 changes: 92 additions & 9 deletions src/coreclr/jit/lower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,14 +437,20 @@ GenTree* Lowering::LowerNode(GenTree* node)
case GT_OR:
case GT_XOR:
{
if (comp->opts.OptimizationEnabled() && node->OperIs(GT_AND))
if (comp->opts.OptimizationEnabled())
{
GenTree* nextNode = nullptr;
if (TryLowerAndNegativeOne(node->AsOp(), &nextNode))
if (node->OperIs(GT_AND) && TryLowerAndNegativeOne(node->AsOp(), &nextNode))
{
return nextNode;
}
assert(nextNode == nullptr);

nextNode = node->gtNext;
if (node->OperIs(GT_AND, GT_OR, GT_XOR) && TryFoldBinop(node->AsOp()))
{
return nextNode;
}
}

return LowerBinaryArithmetic(node->AsOp());
Expand Down Expand Up @@ -538,13 +544,16 @@ GenTree* Lowering::LowerNode(GenTree* node)

case GT_CAST:
{
if (!TryRemoveCast(node->AsCast()))
GenTree* nextNode = node->gtNext;
if (TryRemoveCast(node->AsCast()))
{
GenTree* nextNode = LowerCast(node);
if (nextNode != nullptr)
{
return nextNode;
}
return nextNode;
}

nextNode = LowerCast(node);
if (nextNode != nullptr)
{
return nextNode;
}
}
break;
Expand All @@ -567,8 +576,16 @@ GenTree* Lowering::LowerNode(GenTree* node)

case GT_ROL:
case GT_ROR:
{
GenTree* next = node->gtNext;
if (comp->opts.OptimizationEnabled() && TryFoldBinop(node->AsOp()))
{
return next;
}

LowerRotate(node);
break;
}

#ifndef TARGET_64BIT
case GT_LSH_HI:
Expand All @@ -580,12 +597,20 @@ GenTree* Lowering::LowerNode(GenTree* node)
case GT_LSH:
case GT_RSH:
case GT_RSZ:
{
GenTree* next = node->gtNext;
if (comp->opts.OptimizationEnabled() && TryFoldBinop(node->AsOp()))
{
return next;
}

#if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
LowerShift(node->AsOp());
#else
ContainCheckShiftRotate(node->AsOp());
#endif
break;
}

case GT_STORE_BLK:
if (node->AsBlk()->Data()->IsCall())
Expand Down Expand Up @@ -7752,6 +7777,64 @@ GenTree* Lowering::LowerSignedDivOrMod(GenTree* node)
return node->gtNext;
}

//------------------------------------------------------------------------
// TryFoldBinop: Try removing a binop node by constant folding.
//
// Parameters:
// node - the node
//
// Returns:
// True if the node was removed
//
bool Lowering::TryFoldBinop(GenTreeOp* node)
{
if (node->gtSetFlags())
{
return false;
}

GenTree* op1 = node->gtGetOp1();
GenTree* op2 = node->gtGetOp2();

if (op1->IsIntegralConst() && op2->IsIntegralConst())
{
GenTree* folded = comp->gtFoldExprConst(node);
assert(folded == node);
if (!folded->OperIsConst())
{
return false;
}

BlockRange().Remove(op1);
BlockRange().Remove(op2);
return true;
}

if (node->OperIs(GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR, GT_OR, GT_XOR) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds like what gtFoldExprSpecial does

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume the problem with calling gtFold* apis in Lower is that they must not create new nodes (other than what they return)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am trying to use the lowest level API I can for this to not run into issues calling this from lower... There's probably a better way to factor this folding to make that clear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to have something like

bool gtFoldExprValue32(GenTree* node, int32_t* result);
bool gtFoldExprValue64(GenTree* node, int64_t* result);

and then call that from both gtFoldExprConst and lowering.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I'm going to try changing this here. We should probably refactor gtFold* APIs to give it some idea of what kind of IR it is allowed to create instead, and then use it more directly from lowering.

(op1->IsIntegralConst(0) || op2->IsIntegralConst(0)))
{
GenTree* zeroOp = op1->IsIntegralConst(0) ? op1 : op2;
GenTree* otherOp = zeroOp == op1 ? op2 : op1;

LIR::Use use;
if (BlockRange().TryGetUse(node, &use))
{
use.ReplaceWith(otherOp);
}
else
{
otherOp->SetUnusedValue();
}

BlockRange().Remove(node);
BlockRange().Remove(zeroOp);

return true;
}

return false;
}

//------------------------------------------------------------------------
// LowerShift: Lower shift nodes
//
Expand All @@ -7761,7 +7844,7 @@ GenTree* Lowering::LowerSignedDivOrMod(GenTree* node)
// Notes:
// Remove unnecessary shift count masking, xarch shift instructions
// mask the shift count to 5 bits (or 6 bits for 64 bit operations).

//
void Lowering::LowerShift(GenTreeOp* shift)
{
assert(shift->OperIs(GT_LSH, GT_RSH, GT_RSZ));
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/lower.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ class Lowering final : public Phase
GenTree* LowerStoreLoc(GenTreeLclVarCommon* tree);
void LowerRotate(GenTree* tree);
void LowerShift(GenTreeOp* shift);
bool TryFoldBinop(GenTreeOp* node);
#ifdef FEATURE_HW_INTRINSICS
GenTree* LowerHWIntrinsic(GenTreeHWIntrinsic* node);
void LowerHWIntrinsicCC(GenTreeHWIntrinsic* node, NamedIntrinsic newIntrinsicId, GenCondition condition);
Expand Down
Loading