Skip to content

Commit

Permalink
Introduce FIELD_ADDR and use it for TLS statics and instance class …
Browse files Browse the repository at this point in the history
…fields (#77353)

* Introduce GT_FIELD_ADDR

* Add FIELD_ADDR to fgAddrCouldBeNull

* Add gtNewFieldAddrNode; move gtNewFieldRef

* Implement Windows x86 TLS via FIELD_ADDR

Tested manually to work as well as it did before.

* Silence the IR checker

* Minor code cleanup

* Prepare morph for instance FIELD_ADDRs

* Fix ObjectAllocator

* Use FIELD_ADDR in ld[s]flda import

* Work around morphing issues

With NativeAOT, we can have FIELD_ADDR nodes that are
effectively NOPs (those for the method table pointer field).

This means not all operators that the expansion produces
will be simple (in fact, they can be more or less arbitrary).

This means we cannot simply call "fgMorphSmpOp" as we used to.
Unfortunately, we cannot just call "fgMorphTree" either, because
it propagates assertions on newly created (or, I suppose, bashed)
IND nodes. This creates lots of new cases where GTF_ORDER_SIDEEFF
is applied to these nodes, affecting CQ.

Work around this by calling "fgMorphTree" for non-simple operators
only.
  • Loading branch information
SingleAccretion committed Nov 10, 2022
1 parent 37ef5fb commit 78322ae
Show file tree
Hide file tree
Showing 12 changed files with 633 additions and 545 deletions.
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.cpp
Expand Up @@ -9365,9 +9365,9 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[IND_TGT_HEAP]");
}
if (tree->gtFlags & GTF_IND_TLS_REF)
if (tree->gtFlags & GTF_IND_REQ_ADDR_IN_REG)
{
chars += printf("[IND_TLS_REF]");
chars += printf("[IND_REQ_ADDR_IN_REG]");
}
if (tree->gtFlags & GTF_IND_ASG_LHS)
{
Expand Down
11 changes: 10 additions & 1 deletion src/coreclr/jit/compiler.h
Expand Up @@ -2641,6 +2641,11 @@ class Compiler

GenTreeField* gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj = nullptr, DWORD offset = 0);

GenTreeField* gtNewFieldAddrNode(var_types type,
CORINFO_FIELD_HANDLE fldHnd,
GenTree* obj = nullptr,
DWORD offset = 0);

GenTreeIndexAddr* gtNewIndexAddr(GenTree* arrayOp,
GenTree* indexOp,
var_types elemType,
Expand All @@ -2663,7 +2668,7 @@ class Compiler

GenTreeMDArr* gtNewMDArrLowerBound(GenTree* arrayOp, unsigned dim, unsigned rank, BasicBlock* block);

GenTreeIndir* gtNewIndir(var_types typ, GenTree* addr);
GenTreeIndir* gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags = GTF_EMPTY);

GenTree* gtNewNullCheck(GenTree* addr, BasicBlock* basicBlock);

Expand Down Expand Up @@ -5724,6 +5729,9 @@ class Compiler

private:
GenTree* fgMorphField(GenTree* tree, MorphAddrContext* mac);
GenTree* fgMorphExpandInstanceField(GenTree* tree, MorphAddrContext* mac);
GenTree* fgMorphExpandTlsFieldAddr(GenTree* tree);
GenTree* fgMorphExpandStaticField(GenTree* tree);
bool fgCanFastTailCall(GenTreeCall* call, const char** failReason);
#if FEATURE_FASTTAILCALL
bool fgCallHasMustCopyByrefParameter(GenTreeCall* callee);
Expand Down Expand Up @@ -10846,6 +10854,7 @@ class GenTreeVisitor
case GT_RETURNTRAP:
case GT_NOP:
case GT_FIELD:
case GT_FIELD_ADDR:
case GT_RETURN:
case GT_RETFILT:
case GT_RUNTIMELOOKUP:
Expand Down
62 changes: 6 additions & 56 deletions src/coreclr/jit/compiler.hpp
Expand Up @@ -1069,61 +1069,6 @@ inline GenTree* Compiler::gtNewRuntimeLookup(CORINFO_GENERIC_HANDLE hnd, CorInfo
return node;
}

//------------------------------------------------------------------------
// gtNewFieldRef: a helper for creating GT_FIELD nodes.
//
// Normalizes struct types (for SIMD vectors). Sets GTF_GLOB_REF for fields
// that may be pointing into globally visible memory.
//
// Arguments:
// type - type for the field node
// fldHnd - the field handle
// obj - the instance, an address
// offset - the field offset
//
// Return Value:
// The created node.
//
inline GenTreeField* Compiler::gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHnd, GenTree* obj, DWORD offset)
{
// GT_FIELD nodes are transformed into GT_IND nodes.
assert(GenTree::s_gtNodeSizes[GT_IND] <= GenTree::s_gtNodeSizes[GT_FIELD]);

if (type == TYP_STRUCT)
{
CORINFO_CLASS_HANDLE structHnd;
eeGetFieldType(fldHnd, &structHnd);
type = impNormStructType(structHnd);
}

GenTreeField* fieldNode = new (this, GT_FIELD) GenTreeField(type, obj, fldHnd, offset);

// If "obj" is the address of a local, note that a field of that struct local has been accessed.
if ((obj != nullptr) && obj->OperIs(GT_ADDR) && varTypeIsStruct(obj->AsUnOp()->gtOp1) &&
obj->AsUnOp()->gtOp1->OperIs(GT_LCL_VAR))
{
LclVarDsc* varDsc = lvaGetDesc(obj->AsUnOp()->gtOp1->AsLclVarCommon());

varDsc->lvFieldAccessed = 1;

if (lvaIsImplicitByRefLocal(lvaGetLclNum(varDsc)))
{
// These structs are passed by reference and can easily become global references if those
// references are exposed. We clear out address-exposure information for these parameters
// when they are converted into references in fgRetypeImplicitByRefArgs() so we do not have
// the necessary information in morph to know if these indirections are actually global
// references, so we have to be conservative here.
fieldNode->gtFlags |= GTF_GLOB_REF;
}
}
else
{
fieldNode->gtFlags |= GTF_GLOB_REF;
}

return fieldNode;
}

inline GenTreeIndexAddr* Compiler::gtNewIndexAddr(GenTree* arrayOp,
GenTree* indexOp,
var_types elemType,
Expand Down Expand Up @@ -1267,10 +1212,14 @@ inline GenTreeMDArr* Compiler::gtNewMDArrLowerBound(GenTree* arrayOp, unsigned d
// Return Value:
// New GT_IND node

inline GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr)
inline GenTreeIndir* Compiler::gtNewIndir(var_types typ, GenTree* addr, GenTreeFlags indirFlags)
{
assert((indirFlags & ~GTF_IND_FLAGS) == GTF_EMPTY);

GenTree* indir = gtNewOperNode(GT_IND, typ, addr);
indir->gtFlags |= indirFlags;
indir->SetIndirExceptionFlags(this);

return indir->AsIndir();
}

Expand Down Expand Up @@ -4130,6 +4079,7 @@ void GenTree::VisitOperands(TVisitor visitor)
// Unary operators with an optional operand
case GT_NOP:
case GT_FIELD:
case GT_FIELD_ADDR:
case GT_RETURN:
case GT_RETFILT:
if (this->AsUnOp()->gtOp1 == nullptr)
Expand Down
139 changes: 63 additions & 76 deletions src/coreclr/jit/flowgraph.cpp
Expand Up @@ -936,102 +936,89 @@ GenTreeCall* Compiler::fgGetSharedCCtor(CORINFO_CLASS_HANDLE cls)
//
bool Compiler::fgAddrCouldBeNull(GenTree* addr)
{
addr = addr->gtEffectiveVal();
if (addr->IsIconHandle())
{
return false;
}
else if (addr->OperIs(GT_CNS_STR, GT_CLS_VAR_ADDR))
{
return false;
}
else if (addr->OperIs(GT_INDEX_ADDR))
{
return !addr->AsIndexAddr()->IsNotNull();
}
else if (addr->OperIs(GT_ARR_ADDR))
{
return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0;
}
else if (addr->OperIs(GT_IND))
{
return (addr->gtFlags & GTF_IND_NONNULL) == 0;
}
else if (addr->gtOper == GT_LCL_VAR)
{
unsigned varNum = addr->AsLclVarCommon()->GetLclNum();

if (lvaIsImplicitByRefLocal(varNum))
{
switch (addr->OperGet())
{
case GT_CNS_INT:
return !addr->IsIconHandle();

case GT_CNS_STR:
case GT_ADDR:
case GT_FIELD_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;
}
}
else if (addr->gtOper == GT_ADDR)
{
if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT)
{
GenTree* cns1Tree = addr->AsOp()->gtOp1;
if (!cns1Tree->IsIconHandle())
{
// Indirection of some random constant...
// It is safest just to return true
return true;
}
}

return false; // we can't have a null address
}
else if (addr->gtOper == GT_ADD)
{
if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT)
{
GenTree* cns1Tree = addr->AsOp()->gtOp1;
if (!cns1Tree->IsIconHandle())
case GT_IND:
return (addr->gtFlags & GTF_IND_NONNULL) == 0;

case GT_INDEX_ADDR:
return !addr->AsIndexAddr()->IsNotNull();

case GT_ARR_ADDR:
return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0;

case GT_LCL_VAR:
return !lvaIsImplicitByRefLocal(addr->AsLclVar()->GetLclNum());

case GT_COMMA:
return fgAddrCouldBeNull(addr->AsOp()->gtOp2);

case GT_ADD:
if (addr->AsOp()->gtOp1->gtOper == GT_CNS_INT)
{
if (!fgIsBigOffset(cns1Tree->AsIntCon()->gtIconVal))
GenTree* cns1Tree = addr->AsOp()->gtOp1;
if (!cns1Tree->IsIconHandle())
{
if (!fgIsBigOffset(cns1Tree->AsIntCon()->gtIconVal))
{
// Op1 was an ordinary small constant
return fgAddrCouldBeNull(addr->AsOp()->gtOp2);
}
}
else // Op1 was a handle represented as a constant
{
// Op1 was an ordinary small constant
return fgAddrCouldBeNull(addr->AsOp()->gtOp2);
// Is Op2 also a constant?
if (addr->AsOp()->gtOp2->gtOper == GT_CNS_INT)
{
GenTree* cns2Tree = addr->AsOp()->gtOp2;
// Is this an addition of a handle and constant
if (!cns2Tree->IsIconHandle())
{
if (!fgIsBigOffset(cns2Tree->AsIntCon()->gtIconVal))
{
// Op2 was an ordinary small constant
return false; // we can't have a null address
}
}
}
}
}
else // Op1 was a handle represented as a constant
else
{
// Is Op2 also a constant?
// Op1 is not a constant. What about Op2?
if (addr->AsOp()->gtOp2->gtOper == GT_CNS_INT)
{
GenTree* cns2Tree = addr->AsOp()->gtOp2;
// Is this an addition of a handle and constant
// Is this an addition of a small constant
if (!cns2Tree->IsIconHandle())
{
if (!fgIsBigOffset(cns2Tree->AsIntCon()->gtIconVal))
{
// Op2 was an ordinary small constant
return false; // we can't have a null address
return fgAddrCouldBeNull(addr->AsOp()->gtOp1);
}
}
}
}
}
else
{
// Op1 is not a constant
// What about Op2?
if (addr->AsOp()->gtOp2->gtOper == GT_CNS_INT)
{
GenTree* cns2Tree = addr->AsOp()->gtOp2;
// Is this an addition of a small constant
if (!cns2Tree->IsIconHandle())
{
if (!fgIsBigOffset(cns2Tree->AsIntCon()->gtIconVal))
{
// Op2 was an ordinary small constant
return fgAddrCouldBeNull(addr->AsOp()->gtOp1);
}
}
}
}
break;

default:
break;
}
return true; // default result: addr could be null

return true; // default result: addr could be null.
}

//------------------------------------------------------------------------------
Expand Down

0 comments on commit 78322ae

Please sign in to comment.