diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index a00b4bb2f49c1a..faf2649305335f 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10962,11 +10962,26 @@ GenTree* Compiler::fgMorphCommutative(GenTreeOp* tree) genTreeOps oper = tree->OperGet(); if (!op1->OperIs(oper) || !tree->gtGetOp2()->IsCnsIntOrI() || !op1->gtGetOp2()->IsCnsIntOrI() || - op1->gtGetOp1()->IsCnsIntOrI() || gtIsActiveCSE_Candidate(op1)) + op1->gtGetOp1()->IsCnsIntOrI()) { return nullptr; } + if (!fgGlobalMorph && (op1 != tree->gtGetOp1())) + { + // 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. + 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; @@ -10980,26 +10995,41 @@ GenTree* Compiler::fgMorphCommutative(GenTreeOp* tree) return nullptr; } - GenTree* foldedCns = gtFoldExprConst(gtNewOperNode(oper, cns1->TypeGet(), cns1, cns2)); - if (!foldedCns->IsCnsIntOrI()) + 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()) { // Give up if we can't fold "C1 op C2" return nullptr; } - cns1->gtIconVal = foldedCns->AsIntCon()->IconValue(); - if ((oper == GT_ADD) && foldedCns->IsCnsIntOrI()) + auto foldedCns = folded->AsIntCon(); + + cns1->SetIconValue(foldedCns->IconValue()); + cns1->SetVNsFromNode(foldedCns); + + if (oper == GT_ADD) { - cns1->AsIntCon()->gtFieldSeq = - GetFieldSeqStore()->Append(cns1->AsIntCon()->gtFieldSeq, cns2->AsIntCon()->gtFieldSeq); + // Note that gtFoldExprConst doesn't maintain fieldSeq when folding constant + // trees of TYP_LONG. + cns1->gtFieldSeq = GetFieldSeqStore()->Append(cns1->gtFieldSeq, cns2->gtFieldSeq); } - GenTreeOp* newTree = tree->gtGetOp1()->AsOp(); + op1 = tree->gtGetOp1(); + op1->SetVNsFromNode(tree); + DEBUG_DESTROY_NODE(tree); DEBUG_DESTROY_NODE(cns2); DEBUG_DESTROY_NODE(foldedCns); - INDEBUG(newTree->gtOp2->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); - return newTree; + INDEBUG(cns1->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + + return op1; } /***************************************************************************** diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56935/Runtime_56935.cs b/src/tests/JIT/Regression/JitBlue/Runtime_56935/Runtime_56935.cs new file mode 100644 index 00000000000000..7dcc9bdb0bbae4 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56935/Runtime_56935.cs @@ -0,0 +1,51 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +namespace Runtime_56935 +{ + public class Program + { + static int clsFld; + + public static int Main() + { + int zeroVal = 0; + + // The rhs of the following statement on Arm64 will be transformed into the following tree + // + // N012 ( 27, 14) [000009] ---X-------- * ADD int $42 + // N010 ( 25, 11) [000029] ---X-------- +--* ADD int $40 + // N008 ( 23, 8) [000032] ---X-------- | +--* NEG int $41 + // N007 ( 22, 7) [000008] ---X-------- | | \--* DIV int $42 + // N003 ( 1, 2) [000004] ------------ | | +--* CNS_INT int 1 $42 + // N006 ( 1, 2) [000024] ------------ | | \--* COMMA int $42 + // N004 ( 0, 0) [000022] ------------ | | +--* NOP void $100 + // N005 ( 1, 2) [000023] ------------ | | \--* CNS_INT int 1 $42 + // N009 ( 1, 2) [000028] ------------ | \--* CNS_INT int 1 $42 + // N011 ( 1, 2) [000003] ------------ \--* CNS_INT int 1 $42 + // + // Then, during optValnumCSE() the tree is transformed even further by fgMorphCommutative() + // + // N014 ( 25, 11) [000029] ---X-------- * ADD int $40 + // N012 ( 23, 8) [000032] ---X-------- +--* NEG int $41 + // N011 ( 22, 7) [000008] ---X-------- | \--* DIV int $42 + // N007 ( 1, 2) [000004] ------------ | +--* CNS_INT int 1 $42 + // N010 ( 1, 2) [000024] ------------ | \--* COMMA int $42 + // N008 ( 0, 0) [000022] ------------ | +--* NOP void $100 + // N009 ( 1, 2) [000023] ------------ | \--* CNS_INT int 1 $42 + // N013 ( 1, 2) [000028] ------------ \--* CNS_INT int 2 $42 + // + // The issue is that VN for [000028] has not been updated ($42 corresponds to CnsInt(1)). + // As a result, during optVNAssertionPropCurStmt() the whole tree is **incorrecly** folded to + // + // After constant propagation on [000029]: + // N007 ( 1, 2) [000040] ------------ * CNS_INT int 0 $40 + + clsFld = 1 + (1 % (zeroVal + 1)); + return (clsFld == 1) ? 100 : 0; + } + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_56935/Runtime_56935.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_56935/Runtime_56935.csproj new file mode 100644 index 00000000000000..f3e1cbd44b4041 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_56935/Runtime_56935.csproj @@ -0,0 +1,12 @@ + + + Exe + + + None + True + + + + +