diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 4a798a74cd9d9f..1a5d3a1b716df2 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17866,7 +17866,7 @@ bool Compiler::gtIsStaticGCBaseHelperCall(GenTree* tree) //------------------------------------------------------------------------ // gtCallGetDefinedRetBufLclAddr: -// Get the tree corresponding to the address of the retbuf taht this call defines. +// Get the tree corresponding to the address of the retbuf that this call defines. // // Parameters: // call - The call node diff --git a/src/coreclr/jit/lir.cpp b/src/coreclr/jit/lir.cpp index 212861a1e58612..6fec33642386cc 100644 --- a/src/coreclr/jit/lir.cpp +++ b/src/coreclr/jit/lir.cpp @@ -1557,10 +1557,8 @@ bool LIR::Range::CheckLIR(Compiler* compiler, bool checkUnusedValues) const // Stack arguments do not produce a value, but they are considered children of the call. // It may be useful to remove these from being call operands, but that may also impact // other code that relies on being able to reach all the operands from a call node. - // The GT_NOP case is because sometimes we eliminate stack argument stores as dead, but - // instead of removing them we replace with a NOP. // The argument of a JTRUE doesn't produce a value (just sets a flag). - assert(((node->OperGet() == GT_CALL) && (def->OperIsStore() || def->OperIs(GT_PUTARG_STK, GT_NOP))) || + assert(((node->OperGet() == GT_CALL) && def->OperIs(GT_PUTARG_STK)) || ((node->OperGet() == GT_JTRUE) && (def->TypeGet() == TYP_VOID) && ((def->gtFlags & GTF_SET_FLAGS) != 0))); continue; diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 51996354297a13..517d14fa1bf9fa 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -2205,35 +2205,17 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange) } //--------------------------------------------------------------------- -// fgRemoveDeadStoreSimple - remove a dead store +// fgRemoveDeadStoreLIR - remove a dead store from LIR // -// pTree - GenTree** to local, including store-form local or local addr (post-rationalize) -// varDsc - var that is being stored to -// life - current live tracked vars (maintained as we walk backwards) -// doAgain - out parameter, true if we should restart the statement -// pStmtInfoDirty - should defer the cost computation to the point after the reverse walk is completed? +// store - A store tree +// block - Block that the store is part of // void Compiler::fgRemoveDeadStoreLIR(GenTree* store, BasicBlock* block) { LIR::Range& blockRange = LIR::AsRange(block); - // If the store is marked as a late argument, it is referenced by a call. - // Instead of removing it, bash it to a NOP. - if ((store->gtFlags & GTF_LATE_ARG) != 0) - { - JITDUMP("node is a late arg; replacing with NOP\n"); - store->gtBashToNOP(); - - // NOTE: this is a bit of a hack. We need to keep these nodes around as they are - // referenced by the call, but they're considered side-effect-free non-value-producing - // nodes, so they will be removed if we don't do this. - store->gtFlags |= GTF_ORDER_SIDEEFF; - } - else - { - blockRange.Remove(store); - } - + assert((store->gtFlags & GTF_LATE_ARG) == 0); + blockRange.Remove(store); assert(!opts.MinOpts()); fgStmtRemoved = true; } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 2b2d13326be0b5..7108c284b6b651 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -1343,20 +1343,7 @@ void Lowering::LowerArg(GenTreeCall* call, CallArg* callArg, bool late) JITDUMP("lowering arg : "); DISPNODE(arg); - - // No assignments should remain by Lowering. - assert(!arg->OperIs(GT_ASG)); - assert(!arg->OperIsPutArgStk()); - - // Assignments/stores at this level are not really placing an argument. - // They are setting up temporary locals that will later be placed into - // outgoing regs or stack. - // Note that atomic ops may be stores and still produce a value. - if (!arg->IsValue()) - { - assert((arg->OperIsStore() && !arg->IsValue()) || arg->IsNothingNode() || arg->OperIsCopyBlkOp()); - return; - } + assert(arg->IsValue()); var_types type = arg->TypeGet(); @@ -6425,7 +6412,7 @@ void Lowering::CheckCallArg(GenTree* arg) { if (!arg->IsValue() && !arg->OperIsPutArgStk()) { - assert(arg->OperIsStore() || arg->IsNothingNode() || arg->OperIsCopyBlkOp()); + assert(arg->OperIsStore() || arg->OperIsCopyBlkOp()); return; } diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index bc0032e19b6968..04517a59935822 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -615,6 +615,28 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge RewriteIndir(use); break; + case GT_CALL: + // In linear order we no longer need to retain the stores in early + // args as these have now been sequenced. + for (CallArg& arg : node->AsCall()->gtArgs.EarlyArgs()) + { + if (!arg.GetEarlyNode()->IsValue()) + { + // GTF_LATE_ARG is no longer meaningful. + arg.GetEarlyNode()->gtFlags &= ~GTF_LATE_ARG; + arg.SetEarlyNode(nullptr); + } + } + +#ifdef DEBUG + // The above means that all argument nodes are now true arguments. + for (CallArg& arg : node->AsCall()->gtArgs.Args()) + { + assert((arg.GetEarlyNode() == nullptr) != (arg.GetLateNode() == nullptr)); + } +#endif + break; + case GT_NOP: // fgMorph sometimes inserts NOP nodes between defs and uses // supposedly 'to prevent constant folding'. In this case, remove the @@ -637,8 +659,8 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge if ((sideEffects & GTF_ALL_EFFECT) == 0) { // The LHS has no side effects. Remove it. - // None of the transforms performed herein violate tree order, so isClosed - // should always be true. + // All transformations on pure trees keep their operands in LIR + // and should not violate tree order. assert(isClosed); BlockRange().Delete(comp, m_block, std::move(lhsRange)); @@ -666,8 +688,8 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge if ((sideEffects & GTF_ALL_EFFECT) == 0) { - // None of the transforms performed herein violate tree order, so isClosed - // should always be true. + // All transformations on pure trees keep their operands in + // LIR and should not violate tree order. assert(isClosed); BlockRange().Delete(comp, m_block, std::move(rhsRange));