Skip to content

Commit

Permalink
Use FIELD_ADDR for all ldfldas (#78226)
Browse files Browse the repository at this point in the history
  • Loading branch information
SingleAccretion authored Nov 29, 2022
1 parent 7f5d079 commit 05c6c42
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 230 deletions.
15 changes: 1 addition & 14 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5681,22 +5681,9 @@ class Compiler
// know will be dereferenced. To know that reliance on implicit null checking is sound, we must further know that
// all offsets between the top-level indirection and the bottom are constant, and that their sum is sufficiently
// small; hence the other fields of MorphAddrContext.
enum MorphAddrContextKind
{
MACK_Ind,
MACK_Addr,
};
struct MorphAddrContext
{
MorphAddrContextKind m_kind;
bool m_allConstantOffsets; // Valid only for "m_kind == MACK_Ind". True iff all offsets between
// top-level indirection and here have been constants.
size_t m_totalOffset; // Valid only for "m_kind == MACK_Ind", and if "m_allConstantOffsets" is true.
// In that case, is the sum of those constant offsets.

MorphAddrContext(MorphAddrContextKind kind) : m_kind(kind), m_allConstantOffsets(true), m_totalOffset(0)
{
}
size_t m_totalOffset = 0; // Sum of offsets between the top-level indirection and here (current context).
};

#ifdef FEATURE_SIMD
Expand Down
29 changes: 25 additions & 4 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7584,7 +7584,11 @@ GenTreeField* Compiler::gtNewFieldRef(var_types type, CORINFO_FIELD_HANDLE fldHn
}

//------------------------------------------------------------------------
// gtNewFieldRef: Create a new GT_FIELD_ADDR node.
// gtNewFieldAddrNode: Create a new GT_FIELD_ADDR node.
//
// TODO-ADDR: consider creating a variant of this which would skip various
// no-op constructs (such as struct fields with zero offsets), and fold
// others (LCL_VAR_ADDR + FIELD_ADDR => LCL_FLD_ADDR).
//
// Arguments:
// type - type for the address node
Expand All @@ -7601,10 +7605,27 @@ GenTreeField* Compiler::gtNewFieldAddrNode(var_types type, CORINFO_FIELD_HANDLE

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

// TODO-ADDR: add GTF_EXCEPT handling here and delete it from callers.
// TODO-ADDR: delete this zero-diff quirk.
fieldNode->gtFlags |= GTF_GLOB_REF;
// 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)))
{
// TODO-ADDR: delete this zero-diff quirk.
fieldNode->gtFlags |= GTF_GLOB_REF;
}
}
else
{
// TODO-ADDR: delete this zero-diff quirk.
fieldNode->gtFlags |= GTF_GLOB_REF;
}

// TODO-ADDR: add GTF_EXCEPT handling here and delete it from callers.
return fieldNode;
}

Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4055,6 +4055,7 @@ struct GenTreeField : public GenTreeUnOp
// True if this field is a volatile memory operation.
bool IsVolatile() const
{
assert(((gtFlags & GTF_FLD_VOLATILE) == 0) || OperIs(GT_FIELD));
return (gtFlags & GTF_FLD_VOLATILE) != 0;
}

Expand Down
44 changes: 10 additions & 34 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9524,11 +9524,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
obj = impGetStructAddr(obj, objType, CHECK_SPILL_ALL, true);
}

DWORD typeFlags = info.compCompHnd->getClassAttribs(resolvedToken.hClass);

// TODO-ADDR: use FIELD_ADDR for all fields, not just those of classes.
//
if (isLoadAddress && ((typeFlags & CORINFO_FLG_VALUECLASS) == 0))
if (isLoadAddress)
{
op1 = gtNewFieldAddrNode(varTypeIsGC(obj) ? TYP_BYREF : TYP_I_IMPL, resolvedToken.hField,
obj, fieldInfo.offset);
Expand All @@ -9550,17 +9546,11 @@ void Compiler::impImportBlockCode(BasicBlock* block)
op1->gtFlags |= GTF_EXCEPT;
}

if (StructHasOverlappingFields(typeFlags))
if (StructHasOverlappingFields(info.compCompHnd->getClassAttribs(resolvedToken.hClass)))
{
op1->AsField()->gtFldMayOverlap = true;
}

// Wrap it in a address of operator if necessary.
if (isLoadAddress && op1->OperIs(GT_FIELD))
{
op1 = gtNewOperNode(GT_ADDR, varTypeIsGC(obj) ? TYP_BYREF : TYP_I_IMPL, op1);
}

if (!isLoadAddress && compIsForInlining() &&
impInlineIsGuaranteedThisDerefBeforeAnySideEffects(nullptr, nullptr, obj,
impInlineInfo->inlArgInfo))
Expand Down Expand Up @@ -12938,41 +12928,27 @@ bool Compiler::impIsInvariant(const GenTree* tree)
//
// Remarks:
// This is a variant of GenTree::IsLocalAddrExpr that is more suitable for
// use during import. Unlike that function, this one handles GT_FIELD nodes.
// use during import. Unlike that function, this one handles field nodes.
//
bool Compiler::impIsAddressInLocal(const GenTree* tree, GenTree** lclVarTreeOut)
{
if (tree->gtOper != GT_ADDR)
{
return false;
}

GenTree* op = tree->AsOp()->gtOp1;
while (op->gtOper == GT_FIELD)
const GenTree* op = tree;
while (op->OperIs(GT_FIELD_ADDR) && op->AsField()->IsInstance())
{
op = op->AsField()->GetFldObj();
if (op && op->gtOper == GT_ADDR) // Skip static fields where op will be NULL.
{
op = op->AsOp()->gtOp1;
}
else
{
return false;
}
}

if (op->gtOper == GT_LCL_VAR)
if (op->OperIs(GT_ADDR) && op->AsUnOp()->gtOp1->OperIs(GT_LCL_VAR))
{
if (lclVarTreeOut != nullptr)
{
*lclVarTreeOut = op;
*lclVarTreeOut = op->AsUnOp()->gtOp1;
}

return true;
}
else
{
return false;
}

return false;
}

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

0 comments on commit 05c6c42

Please sign in to comment.