Skip to content

Commit

Permalink
Remove GTF_GLOB_REF on a few more addressing nodes (#68741)
Browse files Browse the repository at this point in the history
GTF_GLOB_REF is necessary on a GT_ADDR node only if the child actually
uses a tree with this flag as a value (and not as a location). This
removes the flag in a few more places by using IsLocalAddrExpr.

Fixes #68669
  • Loading branch information
jakobbotsch authored May 2, 2022
1 parent 463be3a commit ff6af23
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3085,8 +3085,8 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
return GenTree::VisitResult::Continue;
});

// ADDR nodes break the "parent flags >= operands flags" invariant for GTF_GLOB_REF.
if (tree->OperIs(GT_ADDR) && op1->OperIs(GT_LCL_VAR, GT_LCL_FLD))
// Addresses of locals never need GTF_GLOB_REF
if (tree->OperIs(GT_ADDR) && tree->IsLocalAddrExpr())
{
expectedFlags &= ~GTF_GLOB_REF;
}
Expand Down
27 changes: 16 additions & 11 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1769,17 +1769,19 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call)
if (setupArg != nullptr)
{
arg.SetEarlyNode(setupArg);

// Make sure we do not break recognition of retbuf-as-local
// optimization here. If this is hit it indicates that we are
// unnecessarily creating temps for some ret buf addresses, and
// gtCallGetDefinedRetBufLclAddr relies on this not to happen.
noway_assert((arg.GetWellKnownArg() != WellKnownArg::RetBuffer) || !call->IsOptimizingRetBufAsLocal());
}

arg.SetLateNode(defArg);
*lateTail = &arg;
lateTail = &arg.LateNextRef();
}

// Make sure we did not do anything here that stops us from being able to
// find the local ret buf if we are optimizing it.
noway_assert(!call->IsOptimizingRetBufAsLocal() || (comp->gtCallGetDefinedRetBufLclAddr(call) != nullptr));

#ifdef DEBUG
if (comp->verbose)
{
Expand Down Expand Up @@ -11171,10 +11173,10 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
}
}

/* Morphing along with folding and inlining may have changed the
* side effect flags, so we have to reset them
*
* NOTE: Don't reset the exception flags on nodes that may throw */
// Morphing along with folding and inlining may have changed the
// side effect flags, so we have to reset them
//
// NOTE: Don't reset the exception flags on nodes that may throw

assert(tree->gtOper != GT_CALL);

Expand All @@ -11183,11 +11185,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
tree->gtFlags &= ~GTF_CALL;
}

/* Propagate the new flags */
// Propagate the new flags
tree->gtFlags |= (op1->gtFlags & GTF_ALL_EFFECT);

// &aliasedVar doesn't need GTF_GLOB_REF, though alisasedVar does
if (oper == GT_ADDR && (op1->gtOper == GT_LCL_VAR))
// addresses of locals do not need GTF_GLOB_REF, even if the child has
// it (is address exposed). Note that general addressing may still need
// GTF_GLOB_REF, for example if the subtree has a comma that involves a
// global reference.
if (tree->OperIs(GT_ADDR) && ((tree->gtFlags & GTF_GLOB_REF) != 0) && tree->IsLocalAddrExpr())
{
tree->gtFlags &= ~GTF_GLOB_REF;
}
Expand Down

0 comments on commit ff6af23

Please sign in to comment.