Skip to content

JIT: Stop calling morph from CSE #106695

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 6 commits into from
Oct 15, 2024
Merged

JIT: Stop calling morph from CSE #106695

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5f4b078
JIT: Stop calling morph from CSE
jakobbotsch Aug 20, 2024
c66aa44
Merge branch 'main' of github.com:dotnet/runtime into stop-morphing-f…
jakobbotsch Oct 7, 2024
7694374
Fix build
jakobbotsch Oct 7, 2024
7eb20f2
Remove references to `optValnumCSE_phase`
jakobbotsch Oct 14, 2024
b8699b9
Merge branch 'main' of github.com:dotnet/runtime into stop-morphing-f…
jakobbotsch Oct 14, 2024
c6943ed
Fix build
jakobbotsch Oct 14, 2024
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
1 change: 0 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6938,7 +6938,6 @@ class Compiler
TypeProducerKind gtGetTypeProducerKind(GenTree* tree);
bool gtIsTypeHandleToRuntimeTypeHelper(GenTreeCall* call);
bool gtIsTypeHandleToRuntimeTypeHandleHelper(GenTreeCall* call, CorInfoHelpFunc* pHelper = nullptr);
bool gtIsActiveCSE_Candidate(GenTree* tree);

bool gtTreeContainsOper(GenTree* tree, genTreeOps op);
ExceptionSetFlags gtCollectExceptions(GenTree* tree);
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17653,11 +17653,6 @@ bool Compiler::gtIsTypeHandleToRuntimeTypeHandleHelper(GenTreeCall* call, CorInf
return helper != CORINFO_HELP_UNDEF;
}

bool Compiler::gtIsActiveCSE_Candidate(GenTree* tree)
{
return (optValnumCSE_phase && IS_CSE_INDEX(tree->gtCSEnum));
}

//------------------------------------------------------------------------
// gtTreeContainsOper -- check if the tree contains any subtree with the specified oper.
//
Expand Down
67 changes: 8 additions & 59 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,7 @@ GenTree* Compiler::fgMorphExpandCast(GenTreeCast* tree)
// Because there is no IL instruction conv.r4.un, uint/ulong -> float
// casts are always imported as CAST(float <- CAST(double <- uint/ulong)).
// We can eliminate the redundant intermediate cast as an optimization.
else if ((dstType == TYP_FLOAT) && (srcType == TYP_DOUBLE) && oper->OperIs(GT_CAST) &&
!gtIsActiveCSE_Candidate(tree)
else if ((dstType == TYP_FLOAT) && (srcType == TYP_DOUBLE) && oper->OperIs(GT_CAST)
#ifdef TARGET_ARM
&& !varTypeIsLong(oper->AsCast()->CastOp())
#endif
Expand Down Expand Up @@ -3400,16 +3399,6 @@ void Compiler::fgMoveOpsLeft(GenTree* tree)
return;
}

if (gtIsActiveCSE_Candidate(op2))
{
// If we have marked op2 as a CSE candidate,
// we can't perform a commutative reordering
// because any value numbers that we computed for op2
// will be incorrect after performing a commutative reordering
//
return;
}

if (oper == GT_MUL && (op2->gtFlags & GTF_MUL_64RSLT))
{
return;
Expand Down Expand Up @@ -7040,11 +7029,11 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)

// Try to replace CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE with a constant gc handle
// pointing to a frozen segment
if (!gtIsActiveCSE_Candidate(call) && gtIsTypeHandleToRuntimeTypeHelper(call))
if (gtIsTypeHandleToRuntimeTypeHelper(call))
{
GenTree* argNode = call->AsCall()->gtArgs.GetArgByIndex(0)->GetNode();
CORINFO_CLASS_HANDLE hClass = gtGetHelperArgClassHandle(argNode);
if ((hClass != NO_CLASS_HANDLE) && !gtIsActiveCSE_Candidate(argNode))
if (hClass != NO_CLASS_HANDLE)
{
CORINFO_OBJECT_HANDLE ptr = info.compCompHnd->getRuntimeTypePointer(hClass);
if (ptr != NULL)
Expand Down Expand Up @@ -7557,12 +7546,6 @@ GenTreeOp* Compiler::fgMorphCommutative(GenTreeOp* tree)
return nullptr;
}

if (gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(op1))
{
// The optimization removes 'tree' from IR and changes the value of 'op1'.
return nullptr;
}

if (tree->OperMayOverflow() && (tree->gtOverflow() || op1->gtOverflow()))
{
return nullptr;
Expand All @@ -7576,12 +7559,6 @@ GenTreeOp* Compiler::fgMorphCommutative(GenTreeOp* tree)
return nullptr;
}

if (gtIsActiveCSE_Candidate(cns1) || gtIsActiveCSE_Candidate(cns2))
{
// The optimization removes 'cns2' from IR and changes the value of 'cns1'.
return nullptr;
}

GenTree* folded = gtFoldExprConst(gtNewOperNode(oper, cns1->TypeGet(), cns1, cns2));

if (!folded->IsCnsIntOrI())
Expand Down Expand Up @@ -7822,8 +7799,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
}

// Convert DIV to UDIV if both op1 and op2 are known to be never negative
if (!gtIsActiveCSE_Candidate(tree) && varTypeIsIntegral(tree) && op1->IsNeverNegative(this) &&
op2->IsNeverNegative(this))
if (varTypeIsIntegral(tree) && op1->IsNeverNegative(this) && op2->IsNeverNegative(this))
{
assert(tree->OperIs(GT_DIV));
tree->ChangeOper(GT_UDIV, GenTree::PRESERVE_VN);
Expand Down Expand Up @@ -7890,8 +7866,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
}

// Convert MOD to UMOD if both op1 and op2 are known to be never negative
if (!gtIsActiveCSE_Candidate(tree) && varTypeIsIntegral(tree) && op1->IsNeverNegative(this) &&
op2->IsNeverNegative(this))
if (varTypeIsIntegral(tree) && op1->IsNeverNegative(this) && op2->IsNeverNegative(this))
{
assert(tree->OperIs(GT_MOD));
tree->ChangeOper(GT_UMOD, GenTree::PRESERVE_VN);
Expand Down Expand Up @@ -8465,7 +8440,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
}

// op2's value may be changed, so it cannot be a CSE candidate.
if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2))
if (op2->IsIntegralConst())
{
tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp());
oper = tree->OperGet();
Expand Down Expand Up @@ -8706,8 +8681,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
// Consider for example the following expression: NEG(NEG(OP)), where any
// NEG is a CSE candidate. Were we to morph this to just OP, CSE would fail to find
// the original NEG in the statement.
if (op1->OperIs(oper) && opts.OptimizationEnabled() && !gtIsActiveCSE_Candidate(tree) &&
!gtIsActiveCSE_Candidate(op1))
if (op1->OperIs(oper) && opts.OptimizationEnabled())
{
JITDUMP("Remove double negation/not\n")
GenTree* op1op1 = op1->gtGetOp1();
Expand Down Expand Up @@ -9175,11 +9149,6 @@ GenTree* Compiler::fgOptimizeCast(GenTreeCast* cast)
{
GenTree* src = cast->CastOp();

if (gtIsActiveCSE_Candidate(cast) || gtIsActiveCSE_Candidate(src))
{
return cast;
}

// See if we can discard the cast.
if (varTypeIsIntegral(cast) && varTypeIsIntegral(src))
{
Expand Down Expand Up @@ -9297,16 +9266,10 @@ GenTree* Compiler::fgOptimizeCastOnStore(GenTree* store)
if (src->gtOverflow())
return store;

if (gtIsActiveCSE_Candidate(src))
return store;

GenTreeCast* cast = src->AsCast();
var_types castToType = cast->CastToType();
var_types castFromType = cast->CastFromType();

if (gtIsActiveCSE_Candidate(cast->CastOp()))
return store;

if (!varTypeIsSmall(store))
return store;

Expand Down Expand Up @@ -9758,7 +9721,6 @@ GenTree* Compiler::fgOptimizeRelationalComparisonWithConst(GenTreeOp* cmp)
{
assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT));
assert(cmp->gtGetOp2()->IsIntegralConst());
assert(!gtIsActiveCSE_Candidate(cmp->gtGetOp2()));

GenTree* op1 = cmp->gtGetOp1();
GenTreeIntConCommon* op2 = cmp->gtGetOp2()->AsIntConCommon();
Expand Down Expand Up @@ -10426,25 +10388,12 @@ GenTree* Compiler::fgOptimizeHWIntrinsicAssociative(GenTreeHWIntrinsic* tree)
return nullptr;
}

if (gtIsActiveCSE_Candidate(tree) || gtIsActiveCSE_Candidate(effectiveOp1))
{
// In the case op1 is a comma, the optimization removes 'tree' from IR and changes
// the value of op1 and otherwise we're changing the value of 'tree' instead
return nullptr;
}

GenTreeVecCon* cns1 = intrinOp1->Op(2)->AsVecCon();
GenTreeVecCon* cns2 = tree->Op(2)->AsVecCon();

assert(cns1->TypeIs(simdType));
assert(cns2->TypeIs(simdType));

if (gtIsActiveCSE_Candidate(cns1) || gtIsActiveCSE_Candidate(cns2))
{
// The optimization removes 'cns2' from IR and changes the value of 'cns1'.
return nullptr;
}

GenTree* res = gtNewSimdHWIntrinsicNode(simdType, cns1, cns2, intrinsicId, simdBaseJitType, simdSize);
res = gtFoldExprHWIntrinsic(res->AsHWIntrinsic());

Expand Down Expand Up @@ -13220,7 +13169,7 @@ void Compiler::fgMorphStmts(BasicBlock* block)
#endif

/* Check for morphedTree as a GT_COMMA with an unconditional throw */
if (!gtIsActiveCSE_Candidate(morphedTree) && fgIsCommaThrow(morphedTree, true))
if (fgIsCommaThrow(morphedTree, true))
{
/* Use the call as the new stmt */
morphedTree = morphedTree->AsOp()->gtOp1;
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/jit/optcse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5298,10 +5298,9 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate)
//
*link = cse;

assert(m_pCompiler->fgRemoveRestOfBlock == false);

/* re-morph the statement */
m_pCompiler->fgMorphBlockStmt(blk, stmt DEBUGARG("optValnumCSE"));
m_pCompiler->gtSetStmtInfo(stmt);
m_pCompiler->fgSetStmtSeq(stmt);
m_pCompiler->gtUpdateStmtSideEffects(stmt);

} while (lst != nullptr);
}
Expand Down
10 changes: 4 additions & 6 deletions src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3251,7 +3251,7 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
// If 'dstt' is unsigned and one of the operands can be narrowed into 'dsst',
// the result of the GT_AND will also fit into 'dstt' and can be narrowed.
// The same is true if one of the operands is an int const and can be narrowed into 'dsst'.
if (!gtIsActiveCSE_Candidate(op2) && ((op2->gtOper == GT_CNS_INT) || varTypeIsUnsigned(dstt)))
if ((op2->gtOper == GT_CNS_INT) || varTypeIsUnsigned(dstt))
{
if (optNarrowTree(op2, srct, dstt, NoVNPair, false))
{
Expand All @@ -3264,8 +3264,7 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
}
}

if ((opToNarrow == nullptr) && !gtIsActiveCSE_Candidate(op1) &&
((op1->gtOper == GT_CNS_INT) || varTypeIsUnsigned(dstt)))
if ((opToNarrow == nullptr) && ((op1->gtOper == GT_CNS_INT) || varTypeIsUnsigned(dstt)))
{
if (optNarrowTree(op1, srct, dstt, NoVNPair, false))
{
Expand Down Expand Up @@ -3325,8 +3324,7 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
noway_assert(genActualType(tree->gtType) == genActualType(op1->gtType));
noway_assert(genActualType(tree->gtType) == genActualType(op2->gtType));
COMMON_BINOP:
if (gtIsActiveCSE_Candidate(op1) || gtIsActiveCSE_Candidate(op2) ||
!optNarrowTree(op1, srct, dstt, NoVNPair, doit) || !optNarrowTree(op2, srct, dstt, NoVNPair, doit))
if (!optNarrowTree(op1, srct, dstt, NoVNPair, doit) || !optNarrowTree(op2, srct, dstt, NoVNPair, doit))
{
noway_assert(doit == false);
return false;
Expand Down Expand Up @@ -3412,7 +3410,7 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu
return false;

case GT_COMMA:
if (!gtIsActiveCSE_Candidate(op2) && optNarrowTree(op2, srct, dstt, vnpNarrow, doit))
if (optNarrowTree(op2, srct, dstt, vnpNarrow, doit))
{
/* Simply change the type of the tree */

Expand Down
Loading