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

Commit 4be9be9

Browse files
sjsinjuBruceForstall
authored andcommitted
[RyuJIT/ARM32] Implement for GT_STORE_OBJ (#10721)
* Implement lowering for GT_STORE_OBJ In #10657, I commented that the messages for NYI were printed about GT_STORE_OBJ on running the CodeGenBringUpTests. 'Lowering::LowerBlockStore(GenTreeBlk* blkNode)' method implementation is just copied. but after lowering phase, in code generation, codegenarm.cpp, below would be used. ```cpp blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll; ``` ```cpp blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindHelper; ``` ```cpp void CodeGen::genCodeForStoreBlk(GenTreeBlk* blkOp) { if (blkOp->gtBlkOpGcUnsafe) { getEmitter()->emitDisableGC(); } bool isCopyBlk = blkOp->OperIsCopyBlkOp(); switch (blkOp->gtBlkOpKind) { case GenTreeBlk::BlkOpKindHelper: if (isCopyBlk) { genCodeForCpBlk(blkOp); } else { genCodeForInitBlk(blkOp); } break; case GenTreeBlk::BlkOpKindUnroll: if (isCopyBlk) { genCodeForCpBlkUnroll(blkOp); } else { genCodeForInitBlkUnroll(blkOp); } break; default: unreached(); } if (blkOp->gtBlkOpGcUnsafe) { getEmitter()->emitEnableGC(); } } ``` 'genCodeForCpBlk' and 'genCodeForInitBlk' are implemented in ARM/ARM64 by MEMCPY/MEMSET but 'genCodeForCpBlkUnroll' and 'genCodeForInitBlkUnroll' are not implemented both ARM and ARM64. Therefore those need to implement. * Implement NYI : GT_STORE_OBJ is needed of write barriers implementation It was copied from ARM64 and removed codes related with gc write barrier which doesn't support on ARM. * Implement CodeGen::genCodeForCpObj * Refactor some codes * Use INS_OPTS_LDST_POST_INC option for post-indexing When structure is copied, the results of asm codes have been strange. IN0013: 000048 ldr r3, [r1+4] IN0014: 00004A str r3, [r0+4] IN0015: 00004C ldr r3, [r1+4] IN0016: 00004E str r3, [r0+4] It needs that the index would increment or the post-indexing. So I use INS_OPTS_LDST_POST_INC option for post-indexing when the instruction is emitted. * Fix conflicts * Fix conflicts and Apply #11219 I want to merge genCodeForCpObj to codegenarmarch.cpp but the function has modified in #11219. So I decided to keep the code divided now. In future, If modifying the function on ARM is also needed, it would be able to modify. * Fix conflicts * Remove NYI * Fix genCountBits assertion
1 parent 4a321d8 commit 4be9be9

File tree

5 files changed

+125
-19
lines changed

5 files changed

+125
-19
lines changed

src/jit/codegenarm.cpp

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,124 @@ void CodeGen::genCodeForNegNot(GenTree* tree)
820820
genProduceReg(tree);
821821
}
822822

823+
// Generate code for CpObj nodes wich copy structs that have interleaved
824+
// GC pointers.
825+
// For this case we'll generate a sequence of loads/stores in the case of struct
826+
// slots that don't contain GC pointers. The generated code will look like:
827+
// ldr tempReg, [R13, #8]
828+
// str tempReg, [R14, #8]
829+
//
830+
// In the case of a GC-Pointer we'll call the ByRef write barrier helper
831+
// who happens to use the same registers as the previous call to maintain
832+
// the same register requirements and register killsets:
833+
// bl CORINFO_HELP_ASSIGN_BYREF
834+
//
835+
// So finally an example would look like this:
836+
// ldr tempReg, [R13, #8]
837+
// str tempReg, [R14, #8]
838+
// bl CORINFO_HELP_ASSIGN_BYREF
839+
// ldr tempReg, [R13, #8]
840+
// str tempReg, [R14, #8]
841+
// bl CORINFO_HELP_ASSIGN_BYREF
842+
// ldr tempReg, [R13, #8]
843+
// str tempReg, [R14, #8]
844+
void CodeGen::genCodeForCpObj(GenTreeObj* cpObjNode)
845+
{
846+
GenTreePtr dstAddr = cpObjNode->Addr();
847+
GenTreePtr source = cpObjNode->Data();
848+
var_types srcAddrType = TYP_BYREF;
849+
bool sourceIsLocal = false;
850+
regNumber dstReg = REG_NA;
851+
regNumber srcReg = REG_NA;
852+
853+
assert(source->isContained());
854+
if (source->gtOper == GT_IND)
855+
{
856+
GenTree* srcAddr = source->gtGetOp1();
857+
assert(!srcAddr->isContained());
858+
srcAddrType = srcAddr->TypeGet();
859+
}
860+
else
861+
{
862+
noway_assert(source->IsLocal());
863+
sourceIsLocal = true;
864+
}
865+
866+
bool dstOnStack = dstAddr->OperIsLocalAddr();
867+
868+
#ifdef DEBUG
869+
assert(!dstAddr->isContained());
870+
871+
// This GenTree node has data about GC pointers, this means we're dealing
872+
// with CpObj.
873+
assert(cpObjNode->gtGcPtrCount > 0);
874+
#endif // DEBUG
875+
876+
// Consume the operands and get them into the right registers.
877+
// They may now contain gc pointers (depending on their type; gcMarkRegPtrVal will "do the right thing").
878+
genConsumeBlockOp(cpObjNode, REG_WRITE_BARRIER_DST_BYREF, REG_WRITE_BARRIER_SRC_BYREF, REG_NA);
879+
gcInfo.gcMarkRegPtrVal(REG_WRITE_BARRIER_SRC_BYREF, srcAddrType);
880+
gcInfo.gcMarkRegPtrVal(REG_WRITE_BARRIER_DST_BYREF, dstAddr->TypeGet());
881+
882+
// Temp register used to perform the sequence of loads and stores.
883+
regNumber tmpReg = cpObjNode->ExtractTempReg();
884+
assert(genIsValidIntReg(tmpReg));
885+
886+
unsigned slots = cpObjNode->gtSlots;
887+
emitter* emit = getEmitter();
888+
889+
BYTE* gcPtrs = cpObjNode->gtGcPtrs;
890+
891+
// If we can prove it's on the stack we don't need to use the write barrier.
892+
emitAttr attr = EA_PTRSIZE;
893+
if (dstOnStack)
894+
{
895+
for (unsigned i = 0; i < slots; ++i)
896+
{
897+
if (gcPtrs[i] == GCT_GCREF)
898+
attr = EA_GCREF;
899+
else if (gcPtrs[i] == GCT_BYREF)
900+
attr = EA_BYREF;
901+
emit->emitIns_R_R_I(INS_ldr, attr, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
902+
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
903+
emit->emitIns_R_R_I(INS_str, attr, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
904+
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
905+
}
906+
}
907+
else
908+
{
909+
unsigned gcPtrCount = cpObjNode->gtGcPtrCount;
910+
911+
unsigned i = 0;
912+
while (i < slots)
913+
{
914+
switch (gcPtrs[i])
915+
{
916+
case TYPE_GC_NONE:
917+
emit->emitIns_R_R_I(INS_ldr, attr, tmpReg, REG_WRITE_BARRIER_SRC_BYREF, TARGET_POINTER_SIZE,
918+
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
919+
emit->emitIns_R_R_I(INS_str, attr, tmpReg, REG_WRITE_BARRIER_DST_BYREF, TARGET_POINTER_SIZE,
920+
INS_FLAGS_DONT_CARE, INS_OPTS_LDST_POST_INC);
921+
break;
922+
923+
default:
924+
// In the case of a GC-Pointer we'll call the ByRef write barrier helper
925+
genEmitHelperCall(CORINFO_HELP_ASSIGN_BYREF, 0, EA_PTRSIZE);
926+
927+
gcPtrCount--;
928+
break;
929+
}
930+
++i;
931+
}
932+
assert(gcPtrCount == 0);
933+
}
934+
935+
// Clear the gcInfo for registers of source and dest.
936+
// While we normally update GC info prior to the last instruction that uses them,
937+
// these actually live into the helper call.
938+
gcInfo.gcMarkRegSetNpt(RBM_WRITE_BARRIER_SRC_BYREF | RBM_WRITE_BARRIER_DST_BYREF);
939+
}
940+
823941
//------------------------------------------------------------------------
824942
// genCodeForShiftLong: Generates the code sequence for a GenTree node that
825943
// represents a three operand bit shift or rotate operation (<<Hi, >>Lo).

src/jit/codegenarmarch.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2357,18 +2357,12 @@ void CodeGen::genCodeForStoreBlk(GenTreeBlk* blkOp)
23572357
{
23582358
assert(blkOp->OperIs(GT_STORE_OBJ, GT_STORE_DYN_BLK, GT_STORE_BLK));
23592359

2360-
#ifdef _TARGET_ARM_
2361-
NYI_IF(blkOp->OperIs(GT_STORE_OBJ), "GT_STORE_OBJ");
2362-
#endif // _TARGET_ARM_
2363-
2364-
#ifndef _TARGET_ARM_ // NYI for ARM
23652360
if (blkOp->OperIs(GT_STORE_OBJ) && blkOp->OperIsCopyBlkOp())
23662361
{
23672362
assert(blkOp->AsObj()->gtGcPtrCount != 0);
23682363
genCodeForCpObj(blkOp->AsObj());
23692364
return;
23702365
}
2371-
#endif // !_TARGET_ARM_
23722366

23732367
if (blkOp->gtBlkOpGcUnsafe)
23742368
{

src/jit/lowerarmarch.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,6 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
175175
if (blkNode->OperGet() == GT_STORE_OBJ)
176176
{
177177
// CopyObj
178-
179-
NYI_ARM("Lowering for GT_STORE_OBJ isn't implemented");
180-
181-
#ifdef _TARGET_ARM64_
182-
183178
GenTreeObj* objNode = blkNode->AsObj();
184179

185180
unsigned slots = objNode->gtSlots;
@@ -205,8 +200,6 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
205200
#endif
206201

207202
blkNode->gtBlkOpKind = GenTreeBlk::BlkOpKindUnroll;
208-
209-
#endif // _TARGET_ARM64_
210203
}
211204
else
212205
{

src/jit/lsraarmarch.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -784,10 +784,6 @@ void Lowering::TreeNodeInfoInitBlockStore(GenTreeBlk* blkNode)
784784
if (blkNode->OperGet() == GT_STORE_OBJ)
785785
{
786786
// CopyObj
787-
NYI_ARM("GT_STORE_OBJ is needed of write barriers implementation");
788-
789-
#ifdef _TARGET_ARM64_
790-
791787
// We don't need to materialize the struct size but we still need
792788
// a temporary register to perform the sequence of loads and stores.
793789
blkNode->gtLsraInfo.internalIntCount = 1;
@@ -813,8 +809,6 @@ void Lowering::TreeNodeInfoInitBlockStore(GenTreeBlk* blkNode)
813809
{
814810
srcAddrOrFill->gtLsraInfo.setSrcCandidates(l, RBM_WRITE_BARRIER_SRC_BYREF);
815811
}
816-
817-
#endif // _TARGET_ARM64_
818812
}
819813
else
820814
{

src/jit/target.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,13 @@ typedef unsigned short regPairNoSmall; // arm: need 12 bits
13571357
#define RBM_WRITE_BARRIER RBM_R1
13581358
#endif
13591359

1360+
//In the ARM case, registers of write barrier use the normal argument registers.
1361+
#define REG_WRITE_BARRIER_SRC_BYREF REG_ARG_1
1362+
#define RBM_WRITE_BARRIER_SRC_BYREF RBM_ARG_1
1363+
1364+
#define REG_WRITE_BARRIER_DST_BYREF REG_ARG_0
1365+
#define RBM_WRITE_BARRIER_DST_BYREF RBM_ARG_0
1366+
13601367
// GenericPInvokeCalliHelper VASigCookie Parameter
13611368
#define REG_PINVOKE_COOKIE_PARAM REG_R4
13621369
#define RBM_PINVOKE_COOKIE_PARAM RBM_R4

0 commit comments

Comments
 (0)