Skip to content

Commit

Permalink
JIT: Remove reference to "CSE candidate" in morph (dotnet#110218)
Browse files Browse the repository at this point in the history
CSE no longer reinvokes morph, and the checks for CSE were removed in
07f15ef. However, a few comments were left over, so clean those up.
  • Loading branch information
jakobbotsch authored Nov 28, 2024
1 parent a25130b commit 3bf2459
Showing 1 changed file with 3 additions and 16 deletions.
19 changes: 3 additions & 16 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7538,8 +7538,7 @@ GenTreeOp* Compiler::fgMorphCommutative(GenTreeOp* tree)
{
// Since 'tree->gtGetOp1()' can have complex structure (e.g. COMMA(..(COMMA(..,op1)))
// don't run the optimization for such trees outside of global morph.
// Otherwise, there is a chance of violating VNs invariants and/or modifying a tree
// that is an active CSE candidate.
// Otherwise, there is a chance of violating VNs invariants.
return nullptr;
}

Expand Down Expand Up @@ -8395,7 +8394,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA

case GT_EQ:
case GT_NE:
// It is not safe to reorder/delete CSE's
if (op2->IsIntegralConst())
{
tree = fgOptimizeEqualityComparisonWithConst(tree->AsOp());
Expand Down Expand Up @@ -8430,7 +8428,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
op2 = tree->gtGetOp2();
}

// op2's value may be changed, so it cannot be a CSE candidate.
if (op2->IsIntegralConst())
{
tree = fgOptimizeRelationalComparisonWithConst(tree->AsOp());
Expand Down Expand Up @@ -8483,9 +8480,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
goto CM_OVF_OP;
}

// TODO #4104: there are a lot of other places where
// this condition is not checked before transformations.
noway_assert(op2);
if (fgGlobalMorph && !op2->TypeIs(TYP_BYREF))
{
/* Check for "op1 - cns2" , we change it to "op1 + (-cns2)" */
Expand Down Expand Up @@ -8668,10 +8662,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
case GT_NOT:
case GT_NEG:
// Remove double negation/not.
// Note: this is not a safe transformation if "tree" is a CSE candidate.
// 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())
{
JITDUMP("Remove double negation/not\n")
Expand Down Expand Up @@ -8959,9 +8949,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA

assert(oper == tree->gtOper);

// Propagate comma throws.
// If we are in the Valuenum CSE phase then don't morph away anything as these
// nodes may have CSE defs/uses in them.
// Propagate comma throws. Only done in global morph since this does not preserve VNs.
if (fgGlobalMorph && (oper != GT_COLON) &&
/* TODO-ASG-Cleanup: delete this zero-diff quirk */ !GenTree::OperIsStore(oper))
{
Expand Down Expand Up @@ -10358,8 +10346,7 @@ GenTree* Compiler::fgOptimizeHWIntrinsicAssociative(GenTreeHWIntrinsic* tree)
{
// Since 'tree->Op(1)' can have complex structure; e.g. `(.., (.., op1))`
// don't run the optimization for such trees outside of global morph.
// Otherwise, there is a chance of violating VNs invariants and/or modifying a tree
// that is an active CSE candidate.
// Otherwise, there is a chance of violating VNs invariants.
return nullptr;
}

Expand Down

0 comments on commit 3bf2459

Please sign in to comment.