Skip to content
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

Refactor codegen and emit to centralize mov reg, reg handling #52661

Merged
merged 2 commits into from
May 17, 2021
Merged

Refactor codegen and emit to centralize mov reg, reg handling #52661

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
519a7d0
Refactor codegen and emit to centralize `mov reg, reg` handling
tannergooding May 3, 2021
4a127db
Responding to PR feedback
tannergooding May 16, 2021
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
15 changes: 15 additions & 0 deletions src/coreclr/jit/codegen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,21 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

void inst_RV(instruction ins, regNumber reg, var_types type, emitAttr size = EA_UNKNOWN);

void inst_Mov(var_types dstType,
regNumber dstReg,
regNumber srcReg,
bool canSkip,
emitAttr size = EA_UNKNOWN,
insFlags flags = INS_FLAGS_DONT_CARE);

void inst_Mov_Extend(var_types srcType,
bool srcInReg,
regNumber dstReg,
regNumber srcReg,
bool canSkip,
emitAttr size = EA_UNKNOWN,
insFlags flags = INS_FLAGS_DONT_CARE);

void inst_RV_RV(instruction ins,
regNumber reg1,
regNumber reg2,
Expand Down
62 changes: 23 additions & 39 deletions src/coreclr/jit/codegenarm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,15 @@ void CodeGen::instGen_Set_Reg_To_Imm(emitAttr size,

if (GetEmitter()->isLowRegister(reg) && (imm_hi16 == 0xffff) && ((imm_lo16 & 0x8000) == 0x8000))
{
GetEmitter()->emitIns_R_R(INS_sxth, EA_4BYTE, reg, reg);
GetEmitter()->emitIns_Mov(INS_sxth, EA_4BYTE, reg, reg, /* canSkip */ false);
Copy link
Member Author

@tannergooding tannergooding May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these emitIns_Mov calls should "probably" be inst_Mov or inst_Move_Extend calls instead, where the instruction is picked based on the register and type.
I tried, for the most part, to minimize the diff from this change however, and so I kept existing calls directly to emitIns_* where possible.

}
else
{
GetEmitter()->emitIns_R_I(INS_movt, size, reg, imm_hi16);
}

if (flags == INS_FLAGS_SET)
GetEmitter()->emitIns_R_R(INS_mov, size, reg, reg, INS_FLAGS_SET);
GetEmitter()->emitIns_Mov(INS_mov, size, reg, reg, /* canSkip */ false, INS_FLAGS_SET);
}
}

Expand Down Expand Up @@ -260,7 +260,7 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre

float f = forceCastToFloat(constValue);
genSetRegToIcon(tmpReg, *((int*)(&f)));
GetEmitter()->emitIns_R_R(INS_vmov_i2f, EA_4BYTE, targetReg, tmpReg);
GetEmitter()->emitIns_Mov(INS_vmov_i2f, EA_4BYTE, targetReg, tmpReg, /* canSkip */ false);
}
else
{
Expand Down Expand Up @@ -550,7 +550,7 @@ void CodeGen::genLclHeap(GenTree* tree)
inst_JMP(EJ_lo, done);

// Update SP to be at the next page of stack that we will tickle
GetEmitter()->emitIns_R_R(INS_mov, EA_PTRSIZE, REG_SPBASE, regTmp);
GetEmitter()->emitIns_Mov(INS_mov, EA_PTRSIZE, REG_SPBASE, regTmp, /* canSkip */ false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of these are currently /* canSkip */ false because that's how the existing code handled it.

However, almost all of them except for genIntToIntCast are technically elidable (assuming the registers are the same) as they don't do have any special semantics.


// Jump to loop and tickle new stack address
inst_JMP(EJ_jmp, loop);
Expand All @@ -559,7 +559,7 @@ void CodeGen::genLclHeap(GenTree* tree)
genDefineTempLabel(done);

// Now just move the final value to SP
GetEmitter()->emitIns_R_R(INS_mov, EA_PTRSIZE, REG_SPBASE, regCnt);
GetEmitter()->emitIns_Mov(INS_mov, EA_PTRSIZE, REG_SPBASE, regCnt, /* canSkip */ false);

// lastTouchDelta is dynamic, and can be up to a page. So if we have outgoing arg space,
// we're going to assume the worst and probe.
Expand Down Expand Up @@ -591,7 +591,7 @@ void CodeGen::genLclHeap(GenTree* tree)
else // stackAdjustment == 0
{
// Move the final value of SP to regCnt
inst_RV_RV(INS_mov, regCnt, REG_SPBASE);
inst_Mov(TYP_I_IMPL, regCnt, REG_SPBASE, /* canSkip */ false);
}

BAILOUT:
Expand Down Expand Up @@ -926,10 +926,7 @@ void CodeGen::genCodeForShiftLong(GenTree* tree)

regNumber regResult = (oper == GT_LSH_HI) ? regHi : regLo;

if (regResult != tree->GetRegNum())
{
inst_RV_RV(INS_mov, tree->GetRegNum(), regResult, targetType);
}
inst_Mov(targetType, tree->GetRegNum(), regResult, /* canSkip */ true);

if (oper == GT_LSH_HI)
{
Expand Down Expand Up @@ -1015,7 +1012,7 @@ void CodeGen::genCodeForStoreLclFld(GenTreeLclFld* tree)
if (targetType == TYP_FLOAT)
{
regNumber floatAsInt = tree->GetSingleTempReg();
emit->emitIns_R_R(INS_vmov_f2i, EA_4BYTE, floatAsInt, dataReg);
emit->emitIns_Mov(INS_vmov_f2i, EA_4BYTE, floatAsInt, dataReg, /* canSkip */ false);
emit->emitIns_R_R(INS_str, EA_4BYTE, floatAsInt, addr);
}
else
Expand Down Expand Up @@ -1093,11 +1090,9 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* tree)
}
else // store into register (i.e move into register)
{
if (dataReg != targetReg)
{
// Assign into targetReg when dataReg (from op1) is not the same register
inst_RV_RV(ins_Copy(targetType), targetReg, dataReg, targetType);
}
// Assign into targetReg when dataReg (from op1) is not the same register
inst_Mov(targetType, targetReg, dataReg, /* canSkip */ true);

genProduceReg(tree);
}
}
Expand Down Expand Up @@ -1185,13 +1180,13 @@ void CodeGen::genCkfinite(GenTree* treeNode)
// Extract and sign-extend the exponent into an integer register
if (targetType == TYP_FLOAT)
{
emit->emitIns_R_R(INS_vmov_f2i, EA_4BYTE, intReg, fpReg);
emit->emitIns_Mov(INS_vmov_f2i, EA_4BYTE, intReg, fpReg, /* canSkip */ false);
emit->emitIns_R_R_I_I(INS_sbfx, EA_4BYTE, intReg, intReg, 23, 8);
}
else
{
assert(targetType == TYP_DOUBLE);
emit->emitIns_R_R(INS_vmov_f2i, EA_4BYTE, intReg, REG_NEXT(fpReg));
emit->emitIns_Mov(INS_vmov_f2i, EA_4BYTE, intReg, REG_NEXT(fpReg), /* canSkip */ false);
emit->emitIns_R_R_I_I(INS_sbfx, EA_4BYTE, intReg, intReg, 20, 11);
}

Expand All @@ -1200,10 +1195,8 @@ void CodeGen::genCkfinite(GenTree* treeNode)
genJumpToThrowHlpBlk(EJ_eq, SCK_ARITH_EXCPN);

// If it's a finite value, copy it to targetReg
if (targetReg != fpReg)
{
emit->emitIns_R_R(ins_Copy(targetType), emitTypeSize(treeNode), targetReg, fpReg);
}
inst_Mov(targetType, targetReg, fpReg, /* canSkip */ true, emitTypeSize(treeNode));

genProduceReg(treeNode);
}

Expand Down Expand Up @@ -1313,16 +1306,10 @@ void CodeGen::genCodeForStoreInd(GenTreeStoreInd* tree)
noway_assert(data->GetRegNum() != REG_ARG_0);

// addr goes in REG_ARG_0
if (addr->GetRegNum() != REG_ARG_0)
{
inst_RV_RV(INS_mov, REG_ARG_0, addr->GetRegNum(), addr->TypeGet());
}
inst_Mov(addr->TypeGet(), REG_ARG_0, addr->GetRegNum(), /* canSkip */ true);

// data goes in REG_ARG_1
if (data->GetRegNum() != REG_ARG_1)
{
inst_RV_RV(INS_mov, REG_ARG_1, data->GetRegNum(), data->TypeGet());
}
inst_Mov(data->TypeGet(), REG_ARG_1, data->GetRegNum(), /* canSkip */ true);

genGCWriteBarrier(tree, writeBarrierForm);
}
Expand Down Expand Up @@ -1425,10 +1412,7 @@ void CodeGen::genLongToIntCast(GenTree* cast)
}
}

if (dstReg != loSrcReg)
{
inst_RV_RV(INS_mov, dstReg, loSrcReg, TYP_INT, EA_4BYTE);
}
inst_Mov(TYP_INT, dstReg, loSrcReg, /* canSkip */ true);

genProduceReg(cast);
}
Expand Down Expand Up @@ -1491,7 +1475,7 @@ void CodeGen::genIntToFloatCast(GenTree* treeNode)
genConsumeOperands(treeNode->AsOp());

assert(insVcvt != INS_invalid);
GetEmitter()->emitIns_R_R(INS_vmov_i2f, srcSize, treeNode->GetRegNum(), op1->GetRegNum());
GetEmitter()->emitIns_Mov(INS_vmov_i2f, srcSize, treeNode->GetRegNum(), op1->GetRegNum(), /* canSkip */ false);
GetEmitter()->emitIns_R_R(insVcvt, srcSize, treeNode->GetRegNum(), treeNode->GetRegNum());

genProduceReg(treeNode);
Expand Down Expand Up @@ -1556,7 +1540,7 @@ void CodeGen::genFloatToIntCast(GenTree* treeNode)

assert(insVcvt != INS_invalid);
GetEmitter()->emitIns_R_R(insVcvt, dstSize, tmpReg, op1->GetRegNum());
GetEmitter()->emitIns_R_R(INS_vmov_f2i, dstSize, treeNode->GetRegNum(), tmpReg);
GetEmitter()->emitIns_Mov(INS_vmov_f2i, dstSize, treeNode->GetRegNum(), tmpReg, /* canSkip */ false);

genProduceReg(treeNode);
}
Expand Down Expand Up @@ -1778,7 +1762,7 @@ void CodeGen::genProfilingLeaveCallback(unsigned helper)
{
// Has a return value and r0 is in use. For emitting Leave profiler callout we would need r0 for passing
// profiler handle. Therefore, r0 is moved to REG_PROFILER_RETURN_SCRATCH as per contract.
GetEmitter()->emitIns_R_R(INS_mov, attr, REG_PROFILER_RET_SCRATCH, REG_R0);
GetEmitter()->emitIns_Mov(INS_mov, attr, REG_PROFILER_RET_SCRATCH, REG_R0, /* canSkip */ false);
genTransferRegGCState(REG_PROFILER_RET_SCRATCH, REG_R0);
regSet.verifyRegUsed(REG_PROFILER_RET_SCRATCH);
}
Expand All @@ -1802,7 +1786,7 @@ void CodeGen::genProfilingLeaveCallback(unsigned helper)
// Restore state that existed before profiler callback
if (r0InUse)
{
GetEmitter()->emitIns_R_R(INS_mov, attr, REG_R0, REG_PROFILER_RET_SCRATCH);
GetEmitter()->emitIns_Mov(INS_mov, attr, REG_R0, REG_PROFILER_RET_SCRATCH, /* canSkip */ false);
genTransferRegGCState(REG_R0, REG_PROFILER_RET_SCRATCH);
gcInfo.gcMarkRegSetNpt(RBM_PROFILER_RET_SCRATCH);
}
Expand Down Expand Up @@ -1870,7 +1854,7 @@ void CodeGen::genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pIni
regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_ARG);
genEmitHelperCall(CORINFO_HELP_STACK_PROBE, 0, EA_UNKNOWN, REG_STACK_PROBE_HELPER_CALL_TARGET);
compiler->unwindPadding();
GetEmitter()->emitIns_R_R(INS_mov, EA_PTRSIZE, REG_SPBASE, REG_STACK_PROBE_HELPER_ARG);
GetEmitter()->emitIns_Mov(INS_mov, EA_PTRSIZE, REG_SPBASE, REG_STACK_PROBE_HELPER_ARG, /* canSkip */ false);

if ((genRegMask(initReg) & (RBM_STACK_PROBE_HELPER_ARG | RBM_STACK_PROBE_HELPER_CALL_TARGET |
RBM_STACK_PROBE_HELPER_TRASH)) != RBM_NONE)
Expand Down
Loading