Skip to content

Support PUTARG_STK(STRUCT LCL_VAR/LCL_FLD) on x86/Unix x64 #70702

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 7 commits into from
Jun 15, 2022
Merged

Support PUTARG_STK(STRUCT LCL_VAR/LCL_FLD) on x86/Unix x64 #70702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ce4274
Add GenTree::GetLayout
SingleAccretion Jun 1, 2022
bb6d54e
x86/Unix x64: lowering
SingleAccretion Jun 1, 2022
727aa9d
x86/Unix x64: "genConsumePutStructArgStk"
SingleAccretion Feb 9, 2022
eea234d
x86/Unix x64: "genStructPutArgUnroll"
SingleAccretion Feb 9, 2022
cca2a77
x86/Unix x64: codegen asserts
SingleAccretion Apr 27, 2022
6887ff7
x86: "genStructPutArgPush"
SingleAccretion Feb 9, 2022
1004ecf
Unix x64: "genStructPutArgPartialRepMovs"
SingleAccretion Jun 2, 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
8 changes: 4 additions & 4 deletions src/coreclr/jit/codegen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1335,10 +1335,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

void genPutStructArgStk(GenTreePutArgStk* treeNode);

unsigned genMove8IfNeeded(unsigned size, regNumber tmpReg, GenTree* srcAddr, unsigned offset);
unsigned genMove4IfNeeded(unsigned size, regNumber tmpReg, GenTree* srcAddr, unsigned offset);
unsigned genMove2IfNeeded(unsigned size, regNumber tmpReg, GenTree* srcAddr, unsigned offset);
unsigned genMove1IfNeeded(unsigned size, regNumber tmpReg, GenTree* srcAddr, unsigned offset);
unsigned genMove8IfNeeded(unsigned size, regNumber tmpReg, GenTree* src, unsigned offset);
unsigned genMove4IfNeeded(unsigned size, regNumber tmpReg, GenTree* src, unsigned offset);
unsigned genMove2IfNeeded(unsigned size, regNumber tmpReg, GenTree* src, unsigned offset);
unsigned genMove1IfNeeded(unsigned size, regNumber tmpReg, GenTree* src, unsigned offset);
void genCodeForLoadOffset(instruction ins, emitAttr size, regNumber dst, GenTree* base, unsigned offset);
void genStoreRegToStackArg(var_types type, regNumber reg, int offset);
void genStructPutArgRepMovs(GenTreePutArgStk* putArgStkNode);
Expand Down
35 changes: 15 additions & 20 deletions src/coreclr/jit/codegenlinear.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1762,21 +1762,21 @@ void CodeGen::genConsumePutStructArgStk(GenTreePutArgStk* putArgNode,
regNumber sizeReg)
{
// The putArgNode children are always contained. We should not consume any registers.
assert(putArgNode->gtGetOp1()->isContained());
assert(putArgNode->Data()->isContained());

// Get the source address.
GenTree* src = putArgNode->gtGetOp1();
// Get the source.
GenTree* src = putArgNode->Data();
regNumber srcAddrReg = REG_NA;
assert(varTypeIsStruct(src));
assert((src->gtOper == GT_OBJ) || ((src->gtOper == GT_IND && varTypeIsSIMD(src))));
GenTree* srcAddr = src->gtGetOp1();
assert(src->OperIs(GT_OBJ) || src->OperIsLocalRead() || (src->OperIs(GT_IND) && varTypeIsSIMD(src)));

assert(dstReg != REG_NA);
assert(srcReg != REG_NA);

// Consume the registers only if they are not contained or set to REG_NA.
if (srcAddr->GetRegNum() != REG_NA)
// Consume the register for the source address if needed.
if (src->OperIsIndir())
{
genConsumeReg(srcAddr);
srcAddrReg = genConsumeReg(src->AsIndir()->Addr());
}

// If the op1 is already in the dstReg - nothing to do.
Expand All @@ -1798,22 +1798,17 @@ void CodeGen::genConsumePutStructArgStk(GenTreePutArgStk* putArgNode,
}
#endif // !TARGET_X86

if (srcAddr->OperIsLocalAddr())
if (srcAddrReg != REG_NA)
{
// The OperLocalAddr is always contained.
assert(srcAddr->isContained());
const GenTreeLclVarCommon* lclNode = srcAddr->AsLclVarCommon();

// Generate LEA instruction to load the LclVar address in RSI.
// Source is known to be on the stack. Use EA_PTRSIZE.
unsigned int offset = lclNode->GetLclOffs();
GetEmitter()->emitIns_R_S(INS_lea, EA_PTRSIZE, srcReg, lclNode->GetLclNum(), offset);
// Source is not known to be on the stack. Use EA_BYREF.
GetEmitter()->emitIns_Mov(INS_mov, EA_BYREF, srcReg, srcAddrReg, /* canSkip */ true);
}
else
{
assert(srcAddr->GetRegNum() != REG_NA);
// Source is not known to be on the stack. Use EA_BYREF.
GetEmitter()->emitIns_Mov(INS_mov, EA_BYREF, srcReg, srcAddr->GetRegNum(), /* canSkip */ true);
// Generate LEA instruction to load the LclVar address in RSI.
// Source is known to be on the stack. Use EA_PTRSIZE.
GetEmitter()->emitIns_R_S(INS_lea, EA_PTRSIZE, srcReg, src->AsLclVarCommon()->GetLclNum(),
src->AsLclVarCommon()->GetLclOffs());
}

if (sizeReg != REG_NA)
Expand Down
111 changes: 50 additions & 61 deletions src/coreclr/jit/codegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3053,21 +3053,19 @@ void CodeGen::genCodeForInitBlkHelper(GenTreeBlk* initBlkNode)

#ifdef FEATURE_PUT_STRUCT_ARG_STK
// Generate code for a load from some address + offset
// baseNode: tree node which can be either a local address or arbitrary node
// offset: distance from the baseNode from which to load
void CodeGen::genCodeForLoadOffset(instruction ins, emitAttr size, regNumber dst, GenTree* baseNode, unsigned offset)
// base: tree node which can be either a local or an indir
// offset: distance from the "base" location from which to load
//
void CodeGen::genCodeForLoadOffset(instruction ins, emitAttr size, regNumber dst, GenTree* base, unsigned offset)
{
emitter* emit = GetEmitter();

if (baseNode->OperIsLocalAddr())
if (base->OperIsLocalRead())
{
const GenTreeLclVarCommon* lclVar = baseNode->AsLclVarCommon();
offset += lclVar->GetLclOffs();
emit->emitIns_R_S(ins, size, dst, lclVar->GetLclNum(), offset);
GetEmitter()->emitIns_R_S(ins, size, dst, base->AsLclVarCommon()->GetLclNum(),
offset + base->AsLclVarCommon()->GetLclOffs());
}
else
{
emit->emitIns_R_AR(ins, size, dst, baseNode->GetRegNum(), offset);
GetEmitter()->emitIns_R_AR(ins, size, dst, base->AsIndir()->Addr()->GetRegNum(), offset);
}
}
#endif // FEATURE_PUT_STRUCT_ARG_STK
Expand Down Expand Up @@ -3338,7 +3336,7 @@ void CodeGen::genCodeForCpBlkRepMovs(GenTreeBlk* cpBlkNode)
// Arguments:
// size - The size of bytes remaining to be moved
// longTmpReg - The tmp register to be used for the long value
// srcAddr - The address of the source struct
// src - The source struct node (LCL/OBJ)
// offset - The current offset being copied
//
// Return Value:
Expand All @@ -3350,7 +3348,7 @@ void CodeGen::genCodeForCpBlkRepMovs(GenTreeBlk* cpBlkNode)
// On x86, longTmpReg must be an xmm reg; on x64 it must be an integer register.
// This is checked by genStoreRegToStackArg.
//
unsigned CodeGen::genMove8IfNeeded(unsigned size, regNumber longTmpReg, GenTree* srcAddr, unsigned offset)
unsigned CodeGen::genMove8IfNeeded(unsigned size, regNumber longTmpReg, GenTree* src, unsigned offset)
{
#ifdef TARGET_X86
instruction longMovIns = INS_movq;
Expand All @@ -3359,7 +3357,7 @@ unsigned CodeGen::genMove8IfNeeded(unsigned size, regNumber longTmpReg, GenTree*
#endif // !TARGET_X86
if ((size & 8) != 0)
{
genCodeForLoadOffset(longMovIns, EA_8BYTE, longTmpReg, srcAddr, offset);
genCodeForLoadOffset(longMovIns, EA_8BYTE, longTmpReg, src, offset);
genStoreRegToStackArg(TYP_LONG, longTmpReg, offset);
return 8;
}
Expand All @@ -3372,7 +3370,7 @@ unsigned CodeGen::genMove8IfNeeded(unsigned size, regNumber longTmpReg, GenTree*
// Arguments:
// size - The size of bytes remaining to be moved
// intTmpReg - The tmp register to be used for the long value
// srcAddr - The address of the source struct
// src - The source struct node (LCL/OBJ)
// offset - The current offset being copied
//
// Return Value:
Expand All @@ -3384,11 +3382,11 @@ unsigned CodeGen::genMove8IfNeeded(unsigned size, regNumber longTmpReg, GenTree*
// intTmpReg must be an integer register.
// This is checked by genStoreRegToStackArg.
//
unsigned CodeGen::genMove4IfNeeded(unsigned size, regNumber intTmpReg, GenTree* srcAddr, unsigned offset)
unsigned CodeGen::genMove4IfNeeded(unsigned size, regNumber intTmpReg, GenTree* src, unsigned offset)
{
if ((size & 4) != 0)
{
genCodeForLoadOffset(INS_mov, EA_4BYTE, intTmpReg, srcAddr, offset);
genCodeForLoadOffset(INS_mov, EA_4BYTE, intTmpReg, src, offset);
genStoreRegToStackArg(TYP_INT, intTmpReg, offset);
return 4;
}
Expand All @@ -3401,7 +3399,7 @@ unsigned CodeGen::genMove4IfNeeded(unsigned size, regNumber intTmpReg, GenTree*
// Arguments:
// size - The size of bytes remaining to be moved
// intTmpReg - The tmp register to be used for the long value
// srcAddr - The address of the source struct
// src - The source struct node (LCL/OBJ)
// offset - The current offset being copied
//
// Return Value:
Expand All @@ -3413,11 +3411,11 @@ unsigned CodeGen::genMove4IfNeeded(unsigned size, regNumber intTmpReg, GenTree*
// intTmpReg must be an integer register.
// This is checked by genStoreRegToStackArg.
//
unsigned CodeGen::genMove2IfNeeded(unsigned size, regNumber intTmpReg, GenTree* srcAddr, unsigned offset)
unsigned CodeGen::genMove2IfNeeded(unsigned size, regNumber intTmpReg, GenTree* src, unsigned offset)
{
if ((size & 2) != 0)
{
genCodeForLoadOffset(INS_mov, EA_2BYTE, intTmpReg, srcAddr, offset);
genCodeForLoadOffset(INS_mov, EA_2BYTE, intTmpReg, src, offset);
genStoreRegToStackArg(TYP_SHORT, intTmpReg, offset);
return 2;
}
Expand All @@ -3430,7 +3428,7 @@ unsigned CodeGen::genMove2IfNeeded(unsigned size, regNumber intTmpReg, GenTree*
// Arguments:
// size - The size of bytes remaining to be moved
// intTmpReg - The tmp register to be used for the long value
// srcAddr - The address of the source struct
// src - The source struct node (LCL/OBJ)
// offset - The current offset being copied
//
// Return Value:
Expand All @@ -3442,11 +3440,11 @@ unsigned CodeGen::genMove2IfNeeded(unsigned size, regNumber intTmpReg, GenTree*
// intTmpReg must be an integer register.
// This is checked by genStoreRegToStackArg.
//
unsigned CodeGen::genMove1IfNeeded(unsigned size, regNumber intTmpReg, GenTree* srcAddr, unsigned offset)
unsigned CodeGen::genMove1IfNeeded(unsigned size, regNumber intTmpReg, GenTree* src, unsigned offset)
{
if ((size & 1) != 0)
{
genCodeForLoadOffset(INS_mov, EA_1BYTE, intTmpReg, srcAddr, offset);
genCodeForLoadOffset(INS_mov, EA_1BYTE, intTmpReg, src, offset);
genStoreRegToStackArg(TYP_BYTE, intTmpReg, offset);
return 1;
}
Expand All @@ -3470,20 +3468,21 @@ unsigned CodeGen::genMove1IfNeeded(unsigned size, regNumber intTmpReg, GenTree*
//
void CodeGen::genStructPutArgUnroll(GenTreePutArgStk* putArgNode)
{
GenTree* src = putArgNode->AsOp()->gtOp1;
GenTree* src = putArgNode->Data();
// We will never call this method for SIMD types, which are stored directly in genPutStructArgStk().
assert(src->isContained() && src->OperIs(GT_OBJ) && src->TypeIs(TYP_STRUCT));
assert(src->isContained() && src->TypeIs(TYP_STRUCT) && (src->OperIs(GT_OBJ) || src->OperIsLocalRead()));

#ifdef TARGET_X86
assert(!m_pushStkArg);
#endif

if (src->AsOp()->gtOp1->isUsedFromReg())
if (src->OperIs(GT_OBJ))
{
genConsumeReg(src->AsOp()->gtOp1);
genConsumeReg(src->AsObj()->Addr());
}

unsigned loadSize = putArgNode->GetArgLoadSize();
assert(!src->AsObj()->GetLayout()->HasGCPtr() && (loadSize <= CPBLK_UNROLL_LIMIT));
assert(!src->GetLayout(compiler)->HasGCPtr() && (loadSize <= CPBLK_UNROLL_LIMIT));

unsigned offset = 0;
regNumber xmmTmpReg = REG_NA;
Expand Down Expand Up @@ -3518,7 +3517,7 @@ void CodeGen::genStructPutArgUnroll(GenTreePutArgStk* putArgNode)
// this probably needs to be changed.

// Load
genCodeForLoadOffset(INS_movdqu, EA_8BYTE, xmmTmpReg, src->gtGetOp1(), offset);
genCodeForLoadOffset(INS_movdqu, EA_8BYTE, xmmTmpReg, src, offset);
// Store
genStoreRegToStackArg(TYP_STRUCT, xmmTmpReg, offset);

Expand All @@ -3528,10 +3527,10 @@ void CodeGen::genStructPutArgUnroll(GenTreePutArgStk* putArgNode)
// Fill the remainder (15 bytes or less) if there's one.
if ((loadSize % XMM_REGSIZE_BYTES) != 0)
{
offset += genMove8IfNeeded(loadSize, longTmpReg, src->AsOp()->gtOp1, offset);
offset += genMove4IfNeeded(loadSize, intTmpReg, src->AsOp()->gtOp1, offset);
offset += genMove2IfNeeded(loadSize, intTmpReg, src->AsOp()->gtOp1, offset);
offset += genMove1IfNeeded(loadSize, intTmpReg, src->AsOp()->gtOp1, offset);
offset += genMove8IfNeeded(loadSize, longTmpReg, src, offset);
offset += genMove4IfNeeded(loadSize, intTmpReg, src, offset);
offset += genMove2IfNeeded(loadSize, intTmpReg, src, offset);
offset += genMove1IfNeeded(loadSize, intTmpReg, src, offset);
assert(offset == loadSize);
}
}
Expand All @@ -3548,8 +3547,7 @@ void CodeGen::genStructPutArgUnroll(GenTreePutArgStk* putArgNode)
void CodeGen::genStructPutArgRepMovs(GenTreePutArgStk* putArgNode)
{
GenTree* src = putArgNode->gtGetOp1();
assert(src->TypeGet() == TYP_STRUCT);
assert(!src->AsObj()->GetLayout()->HasGCPtr());
assert(src->TypeIs(TYP_STRUCT) && !src->GetLayout(compiler)->HasGCPtr());

// Make sure we got the arguments of the cpblk operation in the right registers, and that
// 'src' is contained as expected.
Expand Down Expand Up @@ -3583,27 +3581,22 @@ void CodeGen::genStructPutArgPush(GenTreePutArgStk* putArgNode)
// future.
assert(m_pushStkArg);

GenTree* src = putArgNode->Data();
GenTree* srcAddr = putArgNode->Data()->AsObj()->Addr();

regNumber srcAddrReg = srcAddr->GetRegNum();
const bool srcAddrInReg = srcAddrReg != REG_NA;

unsigned srcLclNum = 0;
unsigned srcLclOffset = 0;
if (srcAddrInReg)
GenTree* src = putArgNode->Data();
regNumber srcAddrReg = REG_NA;
unsigned srcLclNum = BAD_VAR_NUM;
unsigned srcLclOffs = BAD_LCL_OFFSET;
if (src->OperIsLocalRead())
{
srcAddrReg = genConsumeReg(srcAddr);
assert(src->isContained());
srcLclNum = src->AsLclVarCommon()->GetLclNum();
srcLclOffs = src->AsLclVarCommon()->GetLclOffs();
}
else
{
assert(srcAddr->OperIsLocalAddr());

srcLclNum = srcAddr->AsLclVarCommon()->GetLclNum();
srcLclOffset = srcAddr->AsLclVarCommon()->GetLclOffs();
srcAddrReg = genConsumeReg(src->AsObj()->Addr());
}

ClassLayout* layout = src->AsObj()->GetLayout();
ClassLayout* layout = src->GetLayout(compiler);
const unsigned loadSize = putArgNode->GetArgLoadSize();
assert(((loadSize < XMM_REGSIZE_BYTES) || layout->HasGCPtr()) && ((loadSize % TARGET_POINTER_SIZE) == 0));
const unsigned numSlots = loadSize / TARGET_POINTER_SIZE;
Expand All @@ -3612,13 +3605,13 @@ void CodeGen::genStructPutArgPush(GenTreePutArgStk* putArgNode)
{
emitAttr slotAttr = emitTypeSize(layout->GetGCPtrType(i));
const unsigned byteOffset = i * TARGET_POINTER_SIZE;
if (srcAddrInReg)
if (srcAddrReg != REG_NA)
{
GetEmitter()->emitIns_AR_R(INS_push, slotAttr, REG_NA, srcAddrReg, byteOffset);
}
else
{
GetEmitter()->emitIns_S(INS_push, slotAttr, srcLclNum, srcLclOffset + byteOffset);
GetEmitter()->emitIns_S(INS_push, slotAttr, srcLclNum, srcLclOffs + byteOffset);
}

AddStackLevel(TARGET_POINTER_SIZE);
Expand All @@ -3643,10 +3636,9 @@ void CodeGen::genStructPutArgPartialRepMovs(GenTreePutArgStk* putArgNode)
// They may now contain gc pointers (depending on their type; gcMarkRegPtrVal will "do the right thing").
genConsumePutStructArgStk(putArgNode, REG_RDI, REG_RSI, REG_NA);

GenTreeObj* src = putArgNode->gtGetOp1()->AsObj();
ClassLayout* layout = src->GetLayout();
const bool srcIsLocal = src->Addr()->OperIsLocalAddr();
const emitAttr srcAddrAttr = srcIsLocal ? EA_PTRSIZE : EA_BYREF;
GenTree* src = putArgNode->Data();
ClassLayout* layout = src->GetLayout(compiler);
const emitAttr srcAddrAttr = src->OperIsLocalRead() ? EA_PTRSIZE : EA_BYREF;

#if DEBUG
unsigned numGCSlotsCopied = 0;
Expand Down Expand Up @@ -5533,7 +5525,7 @@ void CodeGen::genCall(GenTreeCall* call)
#ifdef FEATURE_PUT_STRUCT_ARG_STK
if (source->TypeIs(TYP_STRUCT) && !source->OperIs(GT_FIELD_LIST))
{
unsigned loadSize = source->AsObj()->GetLayout()->GetSize();
unsigned loadSize = source->GetLayout(compiler)->GetSize();
assert(argSize == roundUp(loadSize, TARGET_POINTER_SIZE));
}
#endif // FEATURE_PUT_STRUCT_ARG_STK
Expand Down Expand Up @@ -7790,11 +7782,11 @@ bool CodeGen::genAdjustStackForPutArgStk(GenTreePutArgStk* putArgStk)
{
case GenTreePutArgStk::Kind::RepInstr:
case GenTreePutArgStk::Kind::Unroll:
assert(!source->AsObj()->GetLayout()->HasGCPtr());
assert(!source->GetLayout(compiler)->HasGCPtr());
break;

case GenTreePutArgStk::Kind::Push:
assert(source->OperIs(GT_FIELD_LIST) || source->AsObj()->GetLayout()->HasGCPtr() ||
assert(source->OperIs(GT_FIELD_LIST) || source->GetLayout(compiler)->HasGCPtr() ||
(argSize < XMM_REGSIZE_BYTES));
break;

Expand Down Expand Up @@ -8308,14 +8300,11 @@ void CodeGen::genPutStructArgStk(GenTreePutArgStk* putArgStk)

assert(targetType == TYP_STRUCT);

ClassLayout* layout = source->AsObj()->GetLayout();

switch (putArgStk->gtPutArgStkKind)
{
case GenTreePutArgStk::Kind::RepInstr:
genStructPutArgRepMovs(putArgStk);
break;

#ifndef TARGET_X86
case GenTreePutArgStk::Kind::PartialRepInstr:
genStructPutArgPartialRepMovs(putArgStk);
Expand Down
Loading