Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use FIELD_ADDR for all ldfldas #78226

Merged
merged 11 commits into from
Nov 29, 2022
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