Skip to content

Commit

Permalink
JIT: Remove TYP_BLK and TYP_LCLBLK (#83036)
Browse files Browse the repository at this point in the history 
This PR allows TYP_STRUCT locals to have block layouts and replaces uses
of TYP_BLK and TYP_LCLBLK with such locals instead.

There is still an invariant that any struct parameter local (even SIMD) has a non-block layout.

Also fixes a bug related to GS cookie handle with jit32 GC encoder -- it is
not allowed to be at fp+0, so we need to insert some padding in some cases because
now that some GS cookie-requiring locals are TYP_STRUCT they can sometimes be
removed from the frame.

To fix a TP regression, also precache the 0-sized block layout in the layout table. This
layout is used by all non-x86 compilations because the outgoing arg area starts out
as a 0-sized block local that is always allocated.
  • Loading branch information
@jakobbotsch
jakobbotsch authored Mar 9, 2023
1 parent e5fb928 commit 5c7e6d6
Show file tree
Hide file tree
Showing 23 changed files with 291 additions and 325 deletions.
4 changes: 2 additions & 2 deletions src/coreclr/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3042,7 +3042,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
#if defined(UNIX_AMD64_ABI)
if (varTypeIsStruct(varDsc))
{
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetStructHnd();
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetLayout()->GetClassHandle();
assert(typeHnd != nullptr);
SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
compiler->eeGetSystemVAmd64PassStructInRegisterDescriptor(typeHnd, &structDesc);
Expand Down Expand Up @@ -3350,7 +3350,7 @@ void CodeGen::genFnPrologCalleeRegArgs(regNumber xtraReg, bool* pXtraRegClobbere
{
// This must be a SIMD type that's fully enregistered, but is passed as an HFA.
// Each field will be inserted into the same destination register.
assert(varTypeIsSIMD(varDsc) && !compiler->isOpaqueSIMDType(varDsc->GetStructHnd()));
assert(varTypeIsSIMD(varDsc) && !compiler->isOpaqueSIMDType(varDsc->GetLayout()));
assert(regArgTab[argNum].slot <= (int)varDsc->lvHfaSlots());
assert(argNum > 0);
assert(regArgTab[argNum - 1].varNum == varNum);
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/codegenxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6365,7 +6365,7 @@ void CodeGen::genJmpMethod(GenTree* jmp)
#if defined(UNIX_AMD64_ABI)
if (varTypeIsStruct(varDsc))
{
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetStructHnd();
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetLayout()->GetClassHandle();
assert(typeHnd != nullptr);

SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;
Expand Down Expand Up @@ -6415,9 +6415,9 @@ void CodeGen::genJmpMethod(GenTree* jmp)
// Register argument
CLANG_FORMAT_COMMENT_ANCHOR;
#ifdef TARGET_X86
noway_assert(
isRegParamType(genActualType(varDsc->TypeGet())) ||
(varTypeIsStruct(varDsc->TypeGet()) && compiler->isTrivialPointerSizedStruct(varDsc->GetStructHnd())));
noway_assert(isRegParamType(genActualType(varDsc->TypeGet())) ||
((varDsc->TypeGet() == TYP_STRUCT) &&
compiler->isTrivialPointerSizedStruct(varDsc->GetLayout()->GetClassHandle())));
#else
noway_assert(isRegParamType(genActualType(varDsc->TypeGet())));
#endif // TARGET_X86
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10280,6 +10280,10 @@ void Compiler::EnregisterStats::RecordLocal(const LclVarDsc* varDsc)
m_stressPoisonImplicitByrefs++;
break;

case AddressExposedReason::EXTERNALLY_VISIBLE_IMPLICITLY:
m_externallyVisibleImplicitly++;
break;

default:
unreached();
break;
Expand Down Expand Up @@ -10376,5 +10380,6 @@ void Compiler::EnregisterStats::Dump(FILE* fout) const
PRINT_STATS(m_stressLclFld, m_addrExposed);
PRINT_STATS(m_dispatchRetBuf, m_addrExposed);
PRINT_STATS(m_stressPoisonImplicitByrefs, m_addrExposed);
PRINT_STATS(m_externallyVisibleImplicitly, m_addrExposed);
}
#endif // TRACK_ENREG_STATS
57 changes: 36 additions & 21 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,9 @@ enum class AddressExposedReason
STRESS_LCL_FLD, // Stress mode replaces localVar with localFld and makes them addrExposed.
DISPATCH_RET_BUF, // Caller return buffer dispatch.
STRESS_POISON_IMPLICIT_BYREFS, // This is an implicit byref we want to poison.
EXTERNALLY_VISIBLE_IMPLICITLY, // Local is visible externally without explicit escape in JIT IR.
// For example because it is used by GC or is the outgoing arg area
// that belongs to callees.
};

#endif // DEBUG
Expand Down Expand Up @@ -1082,6 +1085,7 @@ class LclVarDsc
lvStkOffs = offset;
}

// TODO-Cleanup: Remove this in favor of GetLayout()->Size/genTypeSize(lvType).
unsigned lvExactSize; // (exact) size of a STRUCT/SIMD/BLK local in bytes.

unsigned lvSize() const;
Expand All @@ -1090,25 +1094,9 @@ class LclVarDsc

unsigned lvSlotNum; // original slot # (if remapped)

// class handle for the local or null if not known or not a class,
// for a struct handle use `GetStructHnd()`.
// class handle for the local or null if not known or not a class
CORINFO_CLASS_HANDLE lvClassHnd;

// Get class handle for a struct local or implicitByRef struct local.
CORINFO_CLASS_HANDLE GetStructHnd() const
{
#ifdef FEATURE_SIMD
if (lvSIMDType && (m_layout == nullptr))
{
return NO_CLASS_HANDLE;
}
#endif

CORINFO_CLASS_HANDLE structHnd = GetLayout()->GetClassHandle();
assert(structHnd != NO_CLASS_HANDLE);
return structHnd;
}

private:
ClassLayout* m_layout; // layout info for structs

Expand Down Expand Up @@ -1191,6 +1179,16 @@ class LclVarDsc
m_layout = layout;
}

// Grow the size of a block layout local.
void GrowBlockLayout(ClassLayout* layout)
{
assert(varTypeIsStruct(lvType));
assert((m_layout == nullptr) || (m_layout->IsBlockLayout() && (m_layout->GetSize() <= layout->GetSize())));
assert(layout->IsBlockLayout());
m_layout = layout;
lvExactSize = layout->GetSize();
}

SsaDefArray<LclSsaVarDsc> lvPerSsaData;

// Returns the address of the per-Ssa data for the given ssaNum (which is required
Expand Down Expand Up @@ -3272,7 +3270,7 @@ class Compiler
// or if the inlinee has GC ref locals.

#if FEATURE_FIXED_OUT_ARGS
unsigned lvaOutgoingArgSpaceVar; // dummy TYP_LCLBLK var for fixed outgoing argument space
unsigned lvaOutgoingArgSpaceVar; // var that represents outgoing argument space
PhasedVar<unsigned> lvaOutgoingArgSpaceSize; // size of fixed outgoing argument space
#endif // FEATURE_FIXED_OUT_ARGS

Expand Down Expand Up @@ -3309,7 +3307,7 @@ class Compiler

#if !defined(FEATURE_EH_FUNCLETS)
// This is used for the callable handlers
unsigned lvaShadowSPslotsVar; // TYP_BLK variable for all the shadow SP slots
unsigned lvaShadowSPslotsVar; // Block-layout TYP_STRUCT variable for all the shadow SP slots
#endif // FEATURE_EH_FUNCLETS

int lvaCachedGenericContextArgOffs;
Expand Down Expand Up @@ -3520,7 +3518,7 @@ class Compiler
bool lvaIsMultiregStruct(LclVarDsc* varDsc, bool isVararg);

// If the local is a TYP_STRUCT, get/set a class handle describing it
CORINFO_CLASS_HANDLE lvaGetStruct(unsigned varNum);
void lvaSetStruct(unsigned varNum, ClassLayout* layout, bool unsafeValueClsCheck);
void lvaSetStruct(unsigned varNum, CORINFO_CLASS_HANDLE typeHnd, bool unsafeValueClsCheck);
void lvaSetStructUsedAsVarArg(unsigned varNum);

Expand Down Expand Up @@ -8715,14 +8713,30 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
return true;
}

bool isOpaqueSIMDType(ClassLayout* layout) const
{
if (layout->IsBlockLayout())
{
return true;
}

return isOpaqueSIMDType(layout->GetClassHandle());
}

// Returns true if the lclVar is an opaque SIMD type.
bool isOpaqueSIMDLclVar(const LclVarDsc* varDsc) const
{
if (!varDsc->lvSIMDType)
{
return false;
}
return isOpaqueSIMDType(varDsc->GetStructHnd());

if (varDsc->GetLayout() == nullptr)
{
return true;
}

return isOpaqueSIMDType(varDsc->GetLayout());
}

bool isNumericsNamespace(const char* ns)
Expand Down Expand Up @@ -10328,6 +10342,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
unsigned m_dispatchRetBuf;
unsigned m_wideIndir;
unsigned m_stressPoisonImplicitByrefs;
unsigned m_externallyVisibleImplicitly;

public:
void RecordLocal(const LclVarDsc* varDsc);
Expand Down
12 changes: 5 additions & 7 deletions src/coreclr/jit/flowgraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1894,9 +1894,8 @@ void Compiler::fgAddReversePInvokeEnterExit()

lvaReversePInvokeFrameVar = lvaGrabTempWithImplicitUse(false DEBUGARG("Reverse Pinvoke FrameVar"));

LclVarDsc* varDsc = lvaGetDesc(lvaReversePInvokeFrameVar);
varDsc->lvType = TYP_BLK;
varDsc->lvExactSize = eeGetEEInfo()->sizeOfReversePInvokeFrame;
LclVarDsc* varDsc = lvaGetDesc(lvaReversePInvokeFrameVar);
lvaSetStruct(lvaReversePInvokeFrameVar, typGetBlkLayout(eeGetEEInfo()->sizeOfReversePInvokeFrame), false);

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

Expand Down Expand Up @@ -2667,9 +2666,8 @@ PhaseStatus Compiler::fgAddInternal()
lvaSetVarAddrExposed(lvaInlinedPInvokeFrameVar DEBUGARG(AddressExposedReason::ESCAPE_ADDRESS));

LclVarDsc* varDsc = lvaGetDesc(lvaInlinedPInvokeFrameVar);
varDsc->lvType = TYP_BLK;
// Make room for the inlined frame.
varDsc->lvExactSize = eeGetEEInfo()->inlinedCallFrameInfo.size;
lvaSetStruct(lvaInlinedPInvokeFrameVar, typGetBlkLayout(eeGetEEInfo()->inlinedCallFrameInfo.size), false);
#if FEATURE_FIXED_OUT_ARGS
// Grab and reserve space for TCB, Frame regs used in PInvoke epilog to pop the inlined frame.
// See genPInvokeMethodEpilog() for use of the grabbed var. This is only necessary if we are
Expand All @@ -2678,8 +2676,7 @@ PhaseStatus Compiler::fgAddInternal()
{
lvaPInvokeFrameRegSaveVar = lvaGrabTempWithImplicitUse(false DEBUGARG("PInvokeFrameRegSave Var"));
varDsc = lvaGetDesc(lvaPInvokeFrameRegSaveVar);
varDsc->lvType = TYP_BLK;
varDsc->lvExactSize = 2 * REGSIZE_BYTES;
lvaSetStruct(lvaPInvokeFrameRegSaveVar, typGetBlkLayout(2 * REGSIZE_BYTES), false);
}
#endif
}
Expand Down Expand Up @@ -3043,6 +3040,7 @@ PhaseStatus Compiler::fgSimpleLowering()
// attempt later will cause an assert.
lvaOutgoingArgSpaceSize = outgoingArgSpaceSize;
lvaOutgoingArgSpaceSize.MarkAsReadOnly();
lvaGetDesc(lvaOutgoingArgSpaceVar)->GrowBlockLayout(typGetBlkLayout(outgoingArgSpaceSize));

#endif // FEATURE_FIXED_OUT_ARGS

Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/gcencode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4183,7 +4183,8 @@ void GCInfo::gcMakeRegPtrTable(

// If this is a TYP_STRUCT, handle its GC pointers.
// Note that the enregisterable struct types cannot have GC pointers in them.
if ((varDsc->TypeGet() == TYP_STRUCT) && varDsc->lvOnFrame && (varDsc->lvExactSize >= TARGET_POINTER_SIZE))
if ((varDsc->TypeGet() == TYP_STRUCT) && varDsc->GetLayout()->HasGCPtr() && varDsc->lvOnFrame &&
(varDsc->lvExactSize >= TARGET_POINTER_SIZE))
{
ClassLayout* layout = varDsc->GetLayout();
unsigned slots = layout->GetSlotCount();
Expand Down
77 changes: 25 additions & 52 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11294,27 +11294,6 @@ void Compiler::gtDispLclVarStructType(unsigned lclNum)
assert(layout != nullptr);
gtDispClassLayout(layout, type);
}
else if (type == TYP_LCLBLK)
{
#if FEATURE_FIXED_OUT_ARGS
assert(lclNum == lvaOutgoingArgSpaceVar);
// Since lvaOutgoingArgSpaceSize is a PhasedVar we can't read it for Dumping until
// after we set it to something.
if (lvaOutgoingArgSpaceSize.HasFinalValue())
{
// A PhasedVar<T> can't be directly used as an arg to a variadic function
unsigned value = lvaOutgoingArgSpaceSize;
printf("<%u> ", value);
}
else
{
printf("<na> "); // The value hasn't yet been determined
}
#else
assert(!"Unknown size");
NO_WAY("Target doesn't support TYP_LCLBLK");
#endif // FEATURE_FIXED_OUT_ARGS
}
}

#if defined(DEBUG) && defined(TARGET_ARM64)
Expand Down Expand Up @@ -11717,7 +11696,7 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack)
else
#endif // !defined(TARGET_64BIT)
{
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetStructHnd();
CORINFO_CLASS_HANDLE typeHnd = varDsc->GetLayout()->GetClassHandle();
CORINFO_FIELD_HANDLE fldHnd =
info.compCompHnd->getFieldInClass(typeHnd, fieldVarDsc->lvFldOrdinal);
fieldName = eeGetFieldName(fldHnd, true, buffer, sizeof(buffer));
Expand Down Expand Up @@ -15811,6 +15790,21 @@ GenTree* Compiler::gtNewTempAssign(
if (dstTyp == TYP_UNDEF)
{
varDsc->lvType = dstTyp = genActualType(valTyp);

if (varTypeIsStruct(dstTyp))
{
if (varTypeIsSIMD(dstTyp))
{
varDsc->lvExactSize = genTypeSize(dstTyp);
#ifdef FEATURE_SIMD
varDsc->lvSIMDType = 1;
#endif
}
else
{
lvaSetStruct(tmp, val->GetLayout(this), false);
}
}
}

#if FEATURE_SIMD
Expand Down Expand Up @@ -15878,40 +15872,13 @@ GenTree* Compiler::gtNewTempAssign(
GenTree* asg;
GenTree* dest = gtNewLclvNode(tmp, dstTyp);

// With first-class structs, we should be propagating the class handle on all non-primitive
// struct types. We don't have a convenient way to do that for all SIMD temps, since some
// internal trees use SIMD types that are not used by the input IL. In this case, we allow
// a null type handle and derive the necessary information about the type from its varType.
CORINFO_CLASS_HANDLE valStructHnd = gtGetStructHandleIfPresent(val);
if (varTypeIsStruct(varDsc) && (valStructHnd == NO_CLASS_HANDLE) && !varTypeIsSIMD(valTyp))
{
// There are some cases where we do not have a struct handle on the return value:
// 1. Handle-less BLK/LCL_FLD nodes.
// 2. The zero constant created by local assertion propagation.
// In these cases, we can use the type of the local for the assignment.
assert(val->gtEffectiveVal(true)->OperIs(GT_IND, GT_BLK, GT_LCL_FLD, GT_CNS_INT));
valStructHnd = lvaGetDesc(tmp)->GetStructHnd();
assert(valStructHnd != NO_CLASS_HANDLE);
}

if ((valStructHnd != NO_CLASS_HANDLE) && val->IsConstInitVal())
if (val->IsConstInitVal())
{
asg = gtNewAssignNode(dest, val);
}
else if (varTypeIsStruct(varDsc) && ((valStructHnd != NO_CLASS_HANDLE) || varTypeIsSIMD(valTyp)))
else if (varTypeIsStruct(varDsc))
{
// The struct value may be be a child of a GT_COMMA due to explicit null checks of indirs/fields.
GenTree* valx = val->gtEffectiveVal(/*commaOnly*/ true);

if (valStructHnd != NO_CLASS_HANDLE)
{
lvaSetStruct(tmp, valStructHnd, false);
}
else
{
assert(valx->gtOper != GT_OBJ);
}

valx->gtFlags |= GTF_DONT_CSE;
asg = impAssignStruct(dest, val, CHECK_SPILL_NONE, pAfterStmt, di, block);
}
Expand Down Expand Up @@ -17703,8 +17670,14 @@ CORINFO_CLASS_HANDLE Compiler::gtGetStructHandleIfPresent(GenTree* tree)
}
break;
case GT_LCL_VAR:
structHnd = lvaGetDesc(tree->AsLclVar())->GetStructHnd();
{
LclVarDsc* dsc = lvaGetDesc(tree->AsLclVar());
if ((dsc->GetLayout() != nullptr) && !dsc->GetLayout()->IsBlockLayout())
{
structHnd = dsc->GetLayout()->GetClassHandle();
}
break;
}
case GT_RETURN:
structHnd = gtGetStructHandleIfPresent(tree->AsOp()->gtOp1);
break;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gschecks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ void Compiler::gsParamsToShadows()
{
// We don't need unsafe value cls check here since we are copying the params and this flag
// would have been set on the original param before reaching here.
lvaSetStruct(shadowVarNum, varDsc->GetStructHnd(), false);
lvaSetStruct(shadowVarNum, varDsc->GetLayout(), false);
shadowVarDsc->lvIsMultiRegArg = varDsc->lvIsMultiRegArg;
shadowVarDsc->lvIsMultiRegRet = varDsc->lvIsMultiRegRet;
}
Expand Down
Loading

0 comments on commit 5c7e6d6

Please sign in to comment.