diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index db1ea44806787..3d61a698a08d5 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -6254,13 +6254,7 @@ GenTree* Compiler::optExtractSideEffListFromConst(GenTree* tree) // Do a sanity check to ensure persistent side effects aren't discarded and // tell gtExtractSideEffList to ignore the root of the tree. // We are relying here on an invariant that VN will only fold non-throwing expressions. - const bool ignoreExceptions = true; - const bool ignoreCctors = false; - // We have to check "AsCall()->HasSideEffects()" here separately because "gtNodeHasSideEffects" - // also checks for side effects that arguments introduce (incosistently so, it otherwise only - // checks for the side effects the node itself has). TODO-Cleanup: change it to not do that? - assert(!gtNodeHasSideEffects(tree, GTF_PERSISTENT_SIDE_EFFECTS) || - (tree->IsCall() && !tree->AsCall()->HasSideEffects(this, ignoreExceptions, ignoreCctors))); + assert(!gtNodeHasSideEffects(tree, GTF_PERSISTENT_SIDE_EFFECTS)); // Exception side effects may be ignored because the root is known to be a constant // (e.g. VN may evaluate a DIV/MOD node to a constant and the node may still diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ee871cfe00a0b..c22804004137e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -16828,31 +16828,7 @@ bool Compiler::gtNodeHasSideEffects(GenTree* tree, GenTreeFlags flags, bool igno { GenTreeCall* const call = potentialCall->AsCall(); const bool ignoreExceptions = (flags & GTF_EXCEPT) == 0; - if (!call->HasSideEffects(this, ignoreExceptions, ignoreCctors)) - { - // If this call is otherwise side effect free, check its arguments. - for (CallArg& arg : call->gtArgs.Args()) - { - // I'm a little worried that args that assign to temps that are late args will look like - // side effects...but better to be conservative for now. - if ((arg.GetEarlyNode() != nullptr) && - gtTreeHasSideEffects(arg.GetEarlyNode(), flags, ignoreCctors)) - { - return true; - } - - if ((arg.GetLateNode() != nullptr) && gtTreeHasSideEffects(arg.GetLateNode(), flags, ignoreCctors)) - { - return true; - } - } - - // Otherwise: - return false; - } - - // Otherwise the GT_CALL is considered to have side-effects. - return true; + return call->HasSideEffects(this, ignoreExceptions, ignoreCctors); } } @@ -16892,19 +16868,30 @@ bool Compiler::gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags /* = GTF_S if (sideEffectFlags == GTF_CALL) { - if (tree->OperGet() == GT_CALL) + if (tree->IsHelperCall()) { // Generally all trees that contain GT_CALL nodes are considered to have side-effects. - // - if (tree->AsCall()->IsHelperCall()) + // However, for some pure helper calls we lie about this. + if (gtNodeHasSideEffects(tree, flags, ignoreCctors)) { - // If this node is a helper call we may not care about the side-effects. - // Note that gtNodeHasSideEffects checks the side effects of the helper itself - // as well as the side effects of its arguments. - return gtNodeHasSideEffects(tree, flags, ignoreCctors); + return true; } + + // The GTF_CALL may be contributed by an operand, so check for + // that. + bool hasCallInOperand = false; + tree->VisitOperands([=, &hasCallInOperand](GenTree* op) { + if (gtTreeHasSideEffects(op, GTF_CALL, ignoreCctors)) + { + hasCallInOperand = true; + return GenTree::VisitResult::Abort; + } + return GenTree::VisitResult::Continue; + }); + + return hasCallInOperand; } - else if (tree->OperGet() == GT_INTRINSIC) + else if (tree->OperIs(GT_INTRINSIC)) { if (gtNodeHasSideEffects(tree, flags, ignoreCctors)) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f773f500697db..d037282e447ab 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1666,6 +1666,8 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call) unsigned tmpVarNum = comp->lvaGrabTemp(true DEBUGARG("argument with side effect")); GenTree* store = comp->gtNewTempStore(tmpVarNum, use.GetNode()); + INDEBUG(store->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + if (setupArg == nullptr) { setupArg = store; @@ -1673,6 +1675,7 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call) else { setupArg = comp->gtNewOperNode(GT_COMMA, TYP_VOID, setupArg, store); + INDEBUG(setupArg->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); } use.SetNode(comp->gtNewLclvNode(tmpVarNum, genActualType(use.GetNode()))); @@ -1688,6 +1691,8 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call) setupArg = comp->gtNewTempStore(tmpVarNum, argx); + INDEBUG(setupArg->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + LclVarDsc* varDsc = comp->lvaGetDesc(tmpVarNum); var_types lclVarType = genActualType(argx->gtType); var_types scalarType = TYP_UNKNOWN;