Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Make StructTypeLayout::GetGCPtrs private again
Browse files Browse the repository at this point in the history 
It's best to access the GC pointer array via accessors like IsGCPtr and GetGCPtrType because they assert that the index is valid.

Also, cleanup duplicated code in copy obj codegen.
  • Loading branch information
@mikedn
mikedn committed Mar 30, 2019
1 parent 5d3d313 commit df25389
Show file tree
Hide file tree
Showing 9 changed files with 128 additions and 255 deletions.
63 changes: 13 additions & 50 deletions src/jit/codegenarm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,13 +697,12 @@ void CodeGen::genCodeForCpObj(GenTreeObj* cpObjNode)

bool dstOnStack = dstAddr->gtSkipReloadOrCopy()->OperIsLocalAddr();

#ifdef DEBUG
assert(!dstAddr->isContained());

// This GenTree node has data about GC pointers, this means we're dealing
// with CpObj.
assert(cpObjNode->GetLayout()->HasGCPtr());
#endif // DEBUG
StructTypeLayout* layout = cpObjNode->GetLayout();
// GT_OBJ should only be used when GC pointers are present.
assert(layout->HasGCPtr());
unsigned slots = layout->GetSlotCount();

// Consume the operands and get them into the right registers.
// They may now contain gc pointers (depending on their type; gcMarkRegPtrVal will "do the right thing").
Expand All @@ -721,62 +720,26 @@ void CodeGen::genCodeForCpObj(GenTreeObj* cpObjNode)
instGen_MemoryBarrier();
}

unsigned slots = cpObjNode->GetLayout()->GetSlotCount();
emitter* emit = getEmitter();
emitter* emit = getEmitter();

const BYTE* gcPtrs = cpObjNode->GetLayout()->GetGCPtrs();

// If we can prove it's on the stack we don't need to use the write barrier.
emitAttr attr = EA_PTRSIZE;
if (dstOnStack)
for (unsigned i = 0; i < slots; i++)
{
for (unsigned i = 0; i < slots; ++i)
if (dstOnStack || !layout->IsGCPtr(i))
{
if (gcPtrs[i] == GCT_GCREF)
{
attr = EA_GCREF;
}
else if (gcPtrs[i] == GCT_BYREF)
{
attr = EA_BYREF;
}
else
{
attr = EA_PTRSIZE;
}
// If the destination is on stack then a GC write barrier
// is not needed but GC pointers must still be reported.
emitAttr attr = emitTypeSize(layout->GetGCPtrType(i));

emit->emitIns_R_R_I(INS_ldr, attr, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
emit->emitIns_R_R_I(INS_str, attr, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
}
}
else
{
unsigned gcPtrCount = cpObjNode->GetLayout()->GetGCPtrCount();

unsigned i = 0;
while (i < slots)
else
{
switch (gcPtrs[i])
{
case TYPE_GC_NONE:
emit->emitIns_R_R_I(INS_ldr, attr, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
emit->emitIns_R_R_I(INS_str, attr, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
break;

default:
// In the case of a GC-Pointer we'll call the ByRef write barrier helper
genEmitHelperCall(CORINFO_HELP_ASSIGN_BYREF, 0, EA_PTRSIZE);

gcPtrCount--;
break;
}
++i;
// In the case of a GC-Pointer we'll call the ByRef write barrier helper
genEmitHelperCall(CORINFO_HELP_ASSIGN_BYREF, 0, EA_PTRSIZE);
}
assert(gcPtrCount == 0);
}

if (cpObjNode->gtFlags & GTF_BLK_VOLATILE)
Expand Down
91 changes: 29 additions & 62 deletions src/jit/codegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2639,7 +2639,10 @@ void CodeGen::genCodeForCpObj(GenTreeObj* cpObjNode)
gcInfo.gcMarkRegPtrVal(REG_WRITE_BARRIER_SRC_BYREF, srcAddrType);
gcInfo.gcMarkRegPtrVal(REG_WRITE_BARRIER_DST_BYREF, dstAddr->TypeGet());

unsigned slots = cpObjNode->GetLayout()->GetSlotCount();
StructTypeLayout* layout = cpObjNode->GetLayout();
// GT_OBJ should only be used when GC pointers are present.
assert(layout->HasGCPtr());
unsigned slots = layout->GetSlotCount();

// Temp register(s) used to perform the sequence of loads and stores.
regNumber tmpReg = cpObjNode->ExtractTempReg();
Expand All @@ -2666,74 +2669,38 @@ void CodeGen::genCodeForCpObj(GenTreeObj* cpObjNode)

emitter* emit = getEmitter();

const BYTE* gcPtrs = cpObjNode->GetLayout()->GetGCPtrs();

// If we can prove it's on the stack we don't need to use the write barrier.
if (dstOnStack)
for (unsigned i = 0; i < slots; i++)
{
unsigned i = 0;
// Check if two or more remaining slots and use a ldp/stp sequence
while (i < slots - 1)
{
emitAttr attr0 = emitTypeSize(compiler->getJitGCType(gcPtrs[i + 0]));
emitAttr attr1 = emitTypeSize(compiler->getJitGCType(gcPtrs[i + 1]));

emit->emitIns_R_R_R_I(INS_ldp, attr0, tmpReg, tmpReg2, REG_WRITE_BARRIER_SRC_BYREF, 2 * TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX, attr1);
emit->emitIns_R_R_R_I(INS_stp, attr0, tmpReg, tmpReg2, REG_WRITE_BARRIER_DST_BYREF, 2 * TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX, attr1);
i += 2;
}

// Use a ldr/str sequence for the last remainder
if (i < slots)
if (dstOnStack || !layout->IsGCPtr(i))
{
emitAttr attr0 = emitTypeSize(compiler->getJitGCType(gcPtrs[i + 0]));
// If the destination is on stack then a GC write barrier
// is not needed but GC pointers must still be reported.
emitAttr attr = emitTypeSize(layout->GetGCPtrType(i + 0));

emit->emitIns_R_R_I(INS_ldr, attr0, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX);
emit->emitIns_R_R_I(INS_str, attr0, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX);
}
}
else
{
unsigned gcPtrCount = cpObjNode->GetLayout()->GetGCPtrCount();

unsigned i = 0;
while (i < slots)
{
switch (gcPtrs[i])
// Check if the next slot's type is also not a GC pointer and use ldp/stp
if ((i + 1 < slots) && (dstOnStack || !layout->IsGCPtr(i + 1)))
{
case TYPE_GC_NONE:
// Check if the next slot's type is also TYP_GC_NONE and use ldp/stp
if ((i + 1 < slots) && (gcPtrs[i + 1] == TYPE_GC_NONE))
{
emit->emitIns_R_R_R_I(INS_ldp, EA_8BYTE, tmpReg, tmpReg2, REG_WRITE_BARRIER_SRC_BYREF,
2 * TARGET_POINTER_SIZE, INS_OPTS_POST_INDEX);
emit->emitIns_R_R_R_I(INS_stp, EA_8BYTE, tmpReg, tmpReg2, REG_WRITE_BARRIER_DST_BYREF,
2 * TARGET_POINTER_SIZE, INS_OPTS_POST_INDEX);
++i; // extra increment of i, since we are copying two items
}
else
{
emit->emitIns_R_R_I(INS_ldr, EA_8BYTE, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX);
emit->emitIns_R_R_I(INS_str, EA_8BYTE, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX);
}
break;

default:
// In the case of a GC-Pointer we'll call the ByRef write barrier helper
genEmitHelperCall(CORINFO_HELP_ASSIGN_BYREF, 0, EA_PTRSIZE);
emitAttr attr2 = emitTypeSize(layout->GetGCPtrType(i + 1));

gcPtrCount--;
break;
emit->emitIns_R_R_R_I(INS_ldp, attr, tmpReg, tmpReg2, REG_WRITE_BARRIER_SRC_BYREF,
2 * TARGET_POINTER_SIZE, INS_OPTS_POST_INDEX, attr2);
emit->emitIns_R_R_R_I(INS_stp, attr, tmpReg, tmpReg2, REG_WRITE_BARRIER_DST_BYREF,
2 * TARGET_POINTER_SIZE, INS_OPTS_POST_INDEX, attr2);
++i; // extra increment of i, since we are copying two items
}
++i;
else
{
emit->emitIns_R_R_I(INS_ldr, attr, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX);
emit->emitIns_R_R_I(INS_str, attr, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
INS_OPTS_POST_INDEX);
}
}
else
{
// In the case of a GC-Pointer we'll call the ByRef write barrier helper
genEmitHelperCall(CORINFO_HELP_ASSIGN_BYREF, 0, EA_PTRSIZE);
}
assert(gcPtrCount == 0);
}

if (cpObjNode->gtFlags & GTF_BLK_VOLATILE)
Expand Down
35 changes: 15 additions & 20 deletions src/jit/codegenarmarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,20 +710,18 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
// the xor ensures that only one of the two is setup, not both
assert((varNode != nullptr) ^ (addrNode != nullptr));

const BYTE* gcPtrs = nullptr;
StructTypeLayout* layout = nullptr;

unsigned gcPtrCount; // The count of GC pointers in the struct
int structSize;
bool isHfa;
int structSize;
bool isHfa;

// This is the varNum for our load operations,
// only used when we have a multireg struct with a LclVar source
unsigned varNumInp = BAD_VAR_NUM;

#ifdef _TARGET_ARM_
// On ARM32, size of reference map can be larger than MAX_ARG_REG_COUNT
gcPtrs = treeNode->GetLayout()->GetGCPtrs();
gcPtrCount = treeNode->GetLayout()->GetGCPtrCount();
layout = treeNode->GetLayout();
#endif
// Setup the structSize, isHFa, and gcPtrCount
if (varNode != nullptr)
Expand All @@ -741,8 +739,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
// as that is how much stack is allocated for this LclVar
isHfa = varDsc->lvIsHfa();
#ifdef _TARGET_ARM64_
gcPtrCount = varDsc->GetLayout()->GetGCPtrCount();
gcPtrs = varDsc->GetLayout()->GetGCPtrs();
layout = varDsc->GetLayout();
#endif // _TARGET_ARM_
}
else // addrNode is used
Expand All @@ -768,16 +765,15 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
structSize = source->AsObj()->GetLayout()->GetSize();
isHfa = compiler->IsHfa(objClass);
#ifdef _TARGET_ARM64_
gcPtrCount = source->AsObj()->GetLayout()->GetGCPtrCount();
gcPtrs = source->AsObj()->GetLayout()->GetGCPtrs();
layout = source->AsObj()->GetLayout();
#endif
}

// If we have an HFA we can't have any GC pointers,
// if not then the max size for the the struct is 16 bytes
if (isHfa)
{
noway_assert(gcPtrCount == 0);
noway_assert(!layout->HasGCPtr());
}
#ifdef _TARGET_ARM64_
else
Expand All @@ -799,8 +795,8 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)

while (remainingSize >= 2 * TARGET_POINTER_SIZE)
{
var_types type0 = compiler->getJitGCType(gcPtrs[nextIndex + 0]);
var_types type1 = compiler->getJitGCType(gcPtrs[nextIndex + 1]);
var_types type0 = layout->GetGCPtrType(nextIndex + 0);
var_types type1 = layout->GetGCPtrType(nextIndex + 1);

if (varNode != nullptr)
{
Expand Down Expand Up @@ -835,7 +831,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
// str r2, [sp, #16]
while (remainingSize >= TARGET_POINTER_SIZE)
{
var_types type = compiler->getJitGCType(gcPtrs[nextIndex]);
var_types type = layout->GetGCPtrType(nextIndex);

if (varNode != nullptr)
{
Expand Down Expand Up @@ -872,7 +868,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
{
if (remainingSize >= TARGET_POINTER_SIZE)
{
var_types nextType = compiler->getJitGCType(gcPtrs[nextIndex]);
var_types nextType = layout->GetGCPtrType(nextIndex);
emitAttr nextAttr = emitTypeSize(nextType);
remainingSize -= TARGET_POINTER_SIZE;

Expand Down Expand Up @@ -905,7 +901,7 @@ void CodeGen::genPutArgStk(GenTreePutArgStk* treeNode)
assert(varNode == nullptr);

// the left over size is smaller than a pointer and thus can never be a GC type
assert(varTypeIsGC(compiler->getJitGCType(gcPtrs[nextIndex])) == false);
assert(!layout->IsGCPtr(nextIndex));

var_types loadType = TYP_UINT;
if (loadSize == 1)
Expand Down Expand Up @@ -1079,9 +1075,8 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
assert((varNode != nullptr) ^ (addrNode != nullptr));

// Setup the structSize, isHFa, and gcPtrCount
const BYTE* gcPtrs = treeNode->GetLayout()->GetGCPtrs();
unsigned gcPtrCount = treeNode->GetLayout()->GetGCPtrCount();
int structSize = treeNode->getArgSize();
StructTypeLayout* layout = treeNode->GetLayout();
int structSize = treeNode->getArgSize();

// This is the varNum for our load operations,
// only used when we have a struct with a LclVar source
Expand Down Expand Up @@ -1129,7 +1124,7 @@ void CodeGen::genPutArgSplit(GenTreePutArgSplit* treeNode)
assert(remainingSize % TARGET_POINTER_SIZE == 0);
while (remainingSize > 0)
{
var_types type = compiler->getJitGCType(gcPtrs[nextIndex]);
var_types type = layout->GetGCPtrType(nextIndex);

if (varNode != nullptr)
{
Expand Down
7 changes: 3 additions & 4 deletions src/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6314,12 +6314,11 @@ void CodeGen::genZeroInitFrame(int untrLclHi, int untrLclLo, regNumber initReg,
(varDsc->lvExactSize >= TARGET_POINTER_SIZE))
{
// We only initialize the GC variables in the TYP_STRUCT
const unsigned slots = (unsigned)compiler->lvaLclSize(varNum) / REGSIZE_BYTES;
const BYTE* gcPtrs = varDsc->GetLayout()->GetGCPtrs();
StructTypeLayout* layout = varDsc->GetLayout();

for (unsigned i = 0; i < slots; i++)
for (unsigned i = 0, slots = layout->GetSlotCount(); i < slots; i++)
{
if (gcPtrs[i] != TYPE_GC_NONE)
if (layout->IsGCPtr(i))
{
getEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE,
genGetZeroReg(initReg, pInitRegZeroed), varNum, i * REGSIZE_BYTES);
Expand Down
Loading

0 comments on commit df25389

Please sign in to comment.