Skip to content

Commit

Permalink
Stop producing and handling ADDR nodes (#78246)
Browse files Browse the repository at this point in the history
* Start importing local address nodes

* Stop producing GT_ADDR nodes

* Use LCL_FLDs in MD array import

* Delete some ADDR uses

* Fix formatting

* Work around MSVC not being smart

Sigh.
  • Loading branch information
SingleAccretion authored Dec 15, 2022
1 parent 0fbdc73 commit 381c782
Show file tree
Hide file tree
Showing 26 changed files with 245 additions and 743 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1781,7 +1781,7 @@ void CodeGen::genConsumePutStructArgStk(GenTreePutArgStk* putArgNode,
}

// If the op1 is already in the dstReg - nothing to do.
// Otherwise load the op1 (GT_ADDR) into the dstReg to copy the struct on the stack by value.
// Otherwise load the op1 (the address) into the dstReg to copy the struct on the stack by value.
CLANG_FORMAT_COMMENT_ANCHOR;

#ifdef TARGET_X86
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -8517,14 +8517,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
(structHandle != m_simdHandleCache->SIMDVector4Handle));
}

// Returns true if the tree corresponds to a TYP_SIMD lcl var.
// Note that both SIMD vector args and locals are mared as lvSIMDType = true, but
// type of an arg node is TYP_BYREF and a local node is TYP_SIMD or TYP_STRUCT.
bool isSIMDTypeLocal(GenTree* tree)
{
return tree->OperIsLocal() && lvaGetDesc(tree->AsLclVarCommon())->lvSIMDType;
}

// Returns true if the lclVar is an opaque SIMD type.
bool isOpaqueSIMDLclVar(const LclVarDsc* varDsc) const
{
Expand Down Expand Up @@ -10821,7 +10813,6 @@ class GenTreeVisitor
case GT_BITCAST:
case GT_CKFINITE:
case GT_LCLHEAP:
case GT_ADDR:
case GT_IND:
case GT_OBJ:
case GT_BLK:
Expand Down
33 changes: 26 additions & 7 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -846,14 +846,34 @@ inline GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree

if (oper == GT_ADDR)
{
if (op1->OperIsIndir())
switch (op1->OperGet())
{
assert(op1->IsValue());
return op1->AsIndir()->Addr();
}
case GT_LCL_VAR:
return gtNewLclVarAddrNode(op1->AsLclVar()->GetLclNum(), type);

case GT_LCL_FLD:
return gtNewLclFldAddrNode(op1->AsLclFld()->GetLclNum(), op1->AsLclFld()->GetLclOffs(), type);

case GT_BLK:
case GT_OBJ:
case GT_IND:
return op1->AsIndir()->Addr();

case GT_FIELD:
{
GenTreeField* fieldAddr =
new (this, GT_FIELD_ADDR) GenTreeField(GT_FIELD_ADDR, type, op1->AsField()->GetFldObj(),
op1->AsField()->gtFldHnd, op1->AsField()->gtFldOffset);
fieldAddr->gtFldMayOverlap = op1->AsField()->gtFldMayOverlap;
#ifdef FEATURE_READYTORUN
fieldAddr->gtFieldLookup = op1->AsField()->gtFieldLookup;
#endif
return fieldAddr;
}

assert(op1->OperIsLocalRead() || op1->OperIs(GT_FIELD));
op1->SetDoNotCSE();
default:
unreached();
}
}

GenTree* node = new (this, oper) GenTreeOp(oper, type, op1, nullptr);
Expand Down Expand Up @@ -4082,7 +4102,6 @@ void GenTree::VisitOperands(TVisitor visitor)
case GT_BITCAST:
case GT_CKFINITE:
case GT_LCLHEAP:
case GT_ADDR:
case GT_IND:
case GT_OBJ:
case GT_BLK:
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3057,7 +3057,6 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
break;

case GT_ASG:
case GT_ADDR:
// Note that this is a weak check - the "op1" location node can be a COMMA.
assert(!op1->CanCSE());
break;
Expand Down Expand Up @@ -3151,12 +3150,6 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
return GenTree::VisitResult::Continue;
});

// Addresses of locals never need GTF_GLOB_REF
if (tree->OperIs(GT_ADDR) && tree->IsLocalAddrExpr())
{
expectedFlags &= ~GTF_GLOB_REF;
}

fgDebugCheckFlagsHelper(tree, actualFlags, expectedFlags);
}

Expand Down
30 changes: 6 additions & 24 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -742,31 +742,18 @@ bool Compiler::fgIsCommaThrow(GenTree* tree, bool forFolding /* = false */)
//
// Return Value:
// If "tree" is a indirection (GT_IND, GT_BLK, or GT_OBJ) whose arg is:
// - an ADDR, whose arg in turn is a LCL_VAR, return that LCL_VAR node;
// - a LCL_VAR_ADDR, return that LCL_VAR_ADDR;
// - else nullptr.
//
// static
GenTreeLclVar* Compiler::fgIsIndirOfAddrOfLocal(GenTree* tree)
{
GenTreeLclVar* res = nullptr;
if (tree->OperIsIndir())
if (tree->OperIsIndir() && tree->AsIndir()->Addr()->OperIs(GT_LCL_VAR_ADDR))
{
GenTree* addr = tree->AsIndir()->Addr();

if (addr->OperGet() == GT_ADDR)
{
GenTree* lclvar = addr->AsOp()->gtOp1;
if (lclvar->OperGet() == GT_LCL_VAR)
{
res = lclvar->AsLclVar();
}
}
else if (addr->OperGet() == GT_LCL_VAR_ADDR)
{
res = addr->AsLclVar();
}
res = tree->AsIndir()->Addr()->AsLclVar();
}

return res;
}

Expand Down Expand Up @@ -932,14 +919,10 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr)
return !addr->IsIconHandle();

case GT_CNS_STR:
case GT_ADDR:
case GT_FIELD_ADDR:
case GT_LCL_VAR_ADDR:
case GT_LCL_FLD_ADDR:
case GT_CLS_VAR_ADDR:
// A GT_ADDR node, by itself, never requires null checking. The expression whose address is being
// taken is either a local or static variable, whose address is necessarily non-null, or else it is
// a field dereference, which will do its own bounds checking if necessary.
return false;

case GT_IND:
Expand Down Expand Up @@ -1842,8 +1825,7 @@ GenTree* Compiler::fgCreateMonitorTree(unsigned lvaMonAcquired, unsigned lvaThis
{
// Insert the expression "enter/exitCrit(this, &acquired)" or "enter/exitCrit(handle, &acquired)"

GenTree* varNode = gtNewLclvNode(lvaMonAcquired, lvaGetDesc(lvaMonAcquired)->TypeGet());
GenTree* varAddrNode = gtNewOperNode(GT_ADDR, TYP_BYREF, varNode);
GenTree* varAddrNode = gtNewLclVarAddrNode(lvaMonAcquired);
GenTree* tree;

if (info.compIsStatic)
Expand Down Expand Up @@ -1978,7 +1960,7 @@ void Compiler::fgAddReversePInvokeEnterExit()

// Add enter pinvoke exit callout at the start of prolog

GenTree* pInvokeFrameVar = gtNewOperNode(GT_ADDR, TYP_I_IMPL, gtNewLclvNode(lvaReversePInvokeFrameVar, TYP_BLK));
GenTree* pInvokeFrameVar = gtNewLclVarAddrNode(lvaReversePInvokeFrameVar);

GenTree* tree;

Expand Down Expand Up @@ -2021,7 +2003,7 @@ void Compiler::fgAddReversePInvokeEnterExit()

// Add reverse pinvoke exit callout at the end of epilog

tree = gtNewOperNode(GT_ADDR, TYP_I_IMPL, gtNewLclvNode(lvaReversePInvokeFrameVar, TYP_BLK));
tree = gtNewLclVarAddrNode(lvaReversePInvokeFrameVar);

CorInfoHelpFunc reversePInvokeExitHelper = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TRACK_TRANSITIONS)
? CORINFO_HELP_JIT_REVERSE_PINVOKE_EXIT_TRACK_TRANSITIONS
Expand Down
6 changes: 2 additions & 4 deletions src/coreclr/jit/forwardsub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,21 +222,19 @@ class ForwardSubVisitor final : public GenTreeVisitor<ForwardSubVisitor>
// Screen out contextual "uses"
//
GenTree* const parent = user;
bool const isAddr = parent->OperIs(GT_ADDR);

bool isCallTarget = false;

// Quirk:
//
// fgGetStubAddrArg cannot handle complex trees (it calls gtClone)
//
bool isCallTarget = false;
if (parent->IsCall())
{
GenTreeCall* const parentCall = parent->AsCall();
isCallTarget = (parentCall->gtCallType == CT_INDIRECT) && (parentCall->gtCallAddr == node);
}

if (!isDef && !isAddr && !isCallTarget)
if (!isDef && !isCallTarget)
{
m_node = node;
m_use = use;
Expand Down
Loading

0 comments on commit 381c782

Please sign in to comment.