From 0765cd8ced21a40feb285cc01d97d621ec3353fb Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 27 Nov 2024 13:25:07 +0100 Subject: [PATCH] JIT: Remove reference to "CSE candidate" in morph 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. --- src/coreclr/jit/morph.cpp | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8583feeea6bb0..70ef34bca62f2 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -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; } @@ -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()); @@ -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()); @@ -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)" */ @@ -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") @@ -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)) { @@ -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; }