Skip to content

Commit

Permalink
Do not spill "mis-sized" struct args passed on stack (dotnet#71399)
Browse files Browse the repository at this point in the history
* Enable a test on Unix x64

* Handle mis-sized structs in LA PUTARG_STK codegen

* Handle mis-sized structs in LA PUTARG_SPLIT codegen

* MisSizedStructs_ArmSplit -> MisSizedStructs_ArmArch

* Add tests

* Do not spill mis-sized stack args

All backends support them.
  • Loading branch information
SingleAccretion authored Jul 5, 2022
1 parent 874043b commit eb123a7
Show file tree
Hide file tree
Showing 5 changed files with 376 additions and 154 deletions.
73 changes: 44 additions & 29 deletions src/coreclr/jit/codegenloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6015,8 +6015,9 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)

while (remainingSize > 0)
{
var_types type;
nextIndex = structOffset / TARGET_POINTER_SIZE;

var_types type;
if (remainingSize >= TARGET_POINTER_SIZE)
{
type = layout->GetGCPtrType(nextIndex);
Expand All @@ -6026,20 +6027,21 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
// the left over size is smaller than a pointer and thus can never be a GC type
assert(!layout->IsGCPtr(nextIndex));

if (remainingSize == 1)
if (remainingSize >= 4)
{
type = TYP_UBYTE;
type = TYP_INT;
}
else if (remainingSize == 2)
else if (remainingSize >= 2)
{
type = TYP_USHORT;
}
else
{
assert(remainingSize == 4);
type = TYP_UINT;
assert(remainingSize == 1);
type = TYP_UBYTE;
}
}

const emitAttr attr = emitTypeSize(type);
const unsigned moveSize = genTypeSize(type);
assert(EA_SIZE_IN_BYTES(attr) == moveSize);
Expand All @@ -6066,7 +6068,6 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area

structOffset += moveSize;
nextIndex++;
}
}
}
Expand Down Expand Up @@ -6225,11 +6226,7 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
}
else // addrNode is used
{
assert(addrNode != nullptr);
// TODO-Cleanup: `Lowering::NewPutArg` marks only `LCL_VAR_ADDR` as contained nowadays,
// Generate code to load the address that we need into a register
genConsumeAddress(addrNode);
addrReg = addrNode->GetRegNum();
addrReg = genConsumeReg(addrNode);

// If addrReg equal to baseReg, we use the last target register as alternative baseReg.
// Because the candidate mask for the internal baseReg does not include any of the target register,
Expand All @@ -6243,39 +6240,57 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
ClassLayout* layout = source->AsObj()->GetLayout();

// Put on stack first
unsigned nextIndex = treeNode->gtNumRegs;
unsigned structOffset = nextIndex * TARGET_POINTER_SIZE;
int remainingSize = treeNode->GetStackByteSize();
unsigned structOffset = treeNode->gtNumRegs * TARGET_POINTER_SIZE;
unsigned remainingSize = layout->GetSize() - structOffset;
unsigned argOffsetOut = treeNode->getArgOffset();

// remainingSize is always multiple of TARGET_POINTER_SIZE
assert(remainingSize % TARGET_POINTER_SIZE == 0);
assert((remainingSize > 0) && (roundUp(remainingSize, TARGET_POINTER_SIZE) == treeNode->GetStackByteSize()));
while (remainingSize > 0)
{
var_types type = layout->GetGCPtrType(nextIndex);
var_types type;
if (remainingSize >= TARGET_POINTER_SIZE)
{
type = layout->GetGCPtrType(structOffset / TARGET_POINTER_SIZE);
}
else if (remainingSize >= 4)
{
type = TYP_INT;
}
else if (remainingSize >= 2)
{
type = TYP_USHORT;
}
else
{
assert(remainingSize == 1);
type = TYP_UBYTE;
}

emitAttr attr = emitActualTypeSize(type);
unsigned moveSize = genTypeSize(type);

instruction loadIns = ins_Load(type);
if (varNode != nullptr)
{
// Load from our varNumImp source
emit->emitIns_R_S(INS_ld_d, emitTypeSize(type), baseReg, srcVarNum, structOffset);
// Load from our local source
emit->emitIns_R_S(loadIns, attr, baseReg, srcVarNum, structOffset);
}
else
{
// check for case of destroying the addrRegister while we still need it
assert(baseReg != addrReg);

// Load from our address expression source
emit->emitIns_R_R_I(INS_ld_d, emitTypeSize(type), baseReg, addrReg, structOffset);
emit->emitIns_R_R_I(loadIns, attr, baseReg, addrReg, structOffset);
}

// Emit str instruction to store the register into the outgoing argument area
emit->emitIns_S_R(INS_st_d, emitTypeSize(type), baseReg, varNumOut, argOffsetOut);
// Emit the instruction to store the register into the outgoing argument area
emit->emitIns_S_R(ins_Store(type), attr, baseReg, varNumOut, argOffsetOut);
argOffsetOut += moveSize;
assert(argOffsetOut <= argOffsetMax);

argOffsetOut += TARGET_POINTER_SIZE; // We stored 4-bytes of the struct
assert(argOffsetOut <= argOffsetMax); // We can't write beyond the outgoing arg area
remainingSize -= TARGET_POINTER_SIZE; // We loaded 4-bytes of the struct
structOffset += TARGET_POINTER_SIZE;
nextIndex += 1;
remainingSize -= moveSize;
structOffset += moveSize;
}

// We set up the registers in order, so that we assign the last target register `baseReg` is no longer in use,
Expand All @@ -6288,7 +6303,7 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)

if (varNode != nullptr)
{
// Load from our varNumImp source
// Load from our local source
emit->emitIns_R_S(ins_Load(type), emitTypeSize(type), targetReg, srcVarNum, structOffset);
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,7 @@ void CallArgs::ArgsComplete(Compiler* comp, GenTreeCall* call)
// TODO-Arm: This optimization is not implemented for ARM32
// so we skip this for ARM32 until it is ported to use RyuJIT backend
//
if (argx->OperGet() == GT_OBJ)
if (argx->OperIs(GT_OBJ) && (arg.AbiInfo.GetRegNum() != REG_STK))
{
GenTreeObj* argObj = argx->AsObj();
unsigned structSize = argObj->GetLayout()->GetSize();
Expand Down
Loading

0 comments on commit eb123a7

Please sign in to comment.