Skip to content

Do not spill "mis-sized" struct args passed on stack #71399

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.

Sign up for GitHub

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

Merged
BruceForstall merged 6 commits into from
Jul 5, 2022
Merged

Do not spill "mis-sized" struct args passed on stack #71399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
15adc78
Enable a test on Unix x64
SingleAccretion Jun 28, 2022
b40ac2b
Handle mis-sized structs in LA PUTARG_STK codegen
SingleAccretion Jun 28, 2022
5270df6
Handle mis-sized structs in LA PUTARG_SPLIT codegen
SingleAccretion Jun 28, 2022
3feddc5
MisSizedStructs_ArmSplit -> MisSizedStructs_ArmArch
SingleAccretion Jun 28, 2022
2142a94
Add tests
SingleAccretion Jun 28, 2022
bd1ead0
Do not spill mis-sized stack args
SingleAccretion Jun 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 44 additions & 29 deletions src/coreclr/jit/codegenloongarch64.cpp
View file
Open in desktop
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
View file
Open in desktop
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