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

Jump to bottom

Fix #70849: Print LARGEJMP instruction on ARM64 #71094

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Fix #70849: Print LARGEJMP instruction on ARM64 #71094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
104 changes: 57 additions & 47 deletions src/coreclr/jit/emitarm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7678,6 +7678,62 @@ void emitter::emitDispInsHelp(
printf("\n");
}

/*****************************************************************************
*
* Handles printing of LARGEJMP pseudo-instruction.
*/

void emitter::emitDispLargeJmp(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* code, size_t sz, insGroup* ig)
{
// Note: don't touch the actual instrDesc. If we accidentally messed it up, it would create a very
// difficult to find bug.

instrDescJmp idJmp;
instrDescJmp* pidJmp = &idJmp;

memset(&idJmp, 0, sizeof(idJmp));

pidJmp->idIns(emitJumpKindToIns(emitReverseJumpKind(emitInsToJumpKind(id->idIns())))); // reverse the
// conditional
// instruction
pidJmp->idInsFmt(IF_T1_K);
pidJmp->idInsSize(emitInsSize(IF_T1_K));
pidJmp->idjShort = 1;
pidJmp->idAddr()->iiaSetInstrCount(1);
pidJmp->idDebugOnlyInfo(id->idDebugOnlyInfo()); // share the idDebugOnlyInfo() field

size_t bcondSizeOrZero = (code == NULL) ? 0 : 2; // branch is 2 bytes
emitDispInsHelp(pidJmp, false, doffs, asmfm, offset, code, bcondSizeOrZero,
NULL /* force display of pc-relative branch */);

code += bcondSizeOrZero;
offset += 2;

// Next, display the unconditional branch

// Reset the local instrDesc
memset(&idJmp, 0, sizeof(idJmp));

pidJmp->idIns(INS_b);
pidJmp->idInsFmt(IF_T2_J2);
pidJmp->idInsSize(emitInsSize(IF_T2_J2));
pidJmp->idjShort = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is set to 1 above and then 0 here. Are both needed? Does pidJmp point to a different piece of memory in these cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

idjShort is initially set to 1 to satisfy an assert in emitDispInsHelp , as we emit a short conditional branch first for the pseudo-instruction. While the following unconditional branch is long (thus suggesting idjShort should be 0), I don't see any asserts in its control flow through emitDispInsHelp that require idjShort == 0, though that could change in the future.

This looks weird because pidJmp is used to emit both the short conditional and the long unconditional branches, but I think the logic is correct.

Copy link
Member

Choose a reason for hiding this comment

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

The memory pointed to gets zeroed between uses (so zeroing isn't required, perhaps, but in this case it's best to be clear and have it).

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, clarity is important. It might be worth a comment to explain that two different things are being done through the ptr but it's not a problem without.

if (id->idIsBound())
{
pidJmp->idSetIsBound();
pidJmp->idAddr()->iiaIGlabel = id->idAddr()->iiaIGlabel;
}
else
{
pidJmp->idAddr()->iiaBBlabel = id->idAddr()->iiaBBlabel;
}
pidJmp->idDebugOnlyInfo(id->idDebugOnlyInfo()); // share the idDebugOnlyInfo() field

size_t brSizeOrZero = (code == NULL) ? 0 : 4; // unconditional branch is 4 bytes
emitDispInsHelp(pidJmp, isNew, doffs, asmfm, offset, code, brSizeOrZero, ig);
}

//--------------------------------------------------------------------
// emitDispIns: Dump the given instruction to jitstdout.
//
Expand Down Expand Up @@ -7714,53 +7770,7 @@ void emitter::emitDispIns(
//
// These instructions don't exist in the actual instruction stream, so we need to fake them
// up to display them.
//
// Note: don't touch the actual instrDesc. If we accidentally messed it up, it would create a very
// difficult to find bug.

instrDescJmp idJmp;
instrDescJmp* pidJmp = &idJmp;

memset(&idJmp, 0, sizeof(idJmp));

pidJmp->idIns(emitJumpKindToIns(emitReverseJumpKind(emitInsToJumpKind(id->idIns())))); // reverse the
// conditional
// instruction
pidJmp->idInsFmt(IF_T1_K);
pidJmp->idInsSize(emitInsSize(IF_T1_K));
pidJmp->idjShort = 1;
pidJmp->idAddr()->iiaSetInstrCount(1);
pidJmp->idDebugOnlyInfo(id->idDebugOnlyInfo()); // share the idDebugOnlyInfo() field

size_t bcondSizeOrZero = (code == NULL) ? 0 : 2; // branch is 2 bytes
emitDispInsHelp(pidJmp, false, doffs, asmfm, offset, code, bcondSizeOrZero,
NULL /* force display of pc-relative branch */);

code += bcondSizeOrZero;
offset += 2;

// Next, display the unconditional branch

// Reset the local instrDesc
memset(&idJmp, 0, sizeof(idJmp));

pidJmp->idIns(INS_b);
pidJmp->idInsFmt(IF_T2_J2);
pidJmp->idInsSize(emitInsSize(IF_T2_J2));
pidJmp->idjShort = 0;
if (id->idIsBound())
{
pidJmp->idSetIsBound();
pidJmp->idAddr()->iiaIGlabel = id->idAddr()->iiaIGlabel;
}
else
{
pidJmp->idAddr()->iiaBBlabel = id->idAddr()->iiaBBlabel;
}
pidJmp->idDebugOnlyInfo(id->idDebugOnlyInfo()); // share the idDebugOnlyInfo() field

size_t brSizeOrZero = (code == NULL) ? 0 : 4; // unconditional branch is 4 bytes
emitDispInsHelp(pidJmp, isNew, doffs, asmfm, offset, code, brSizeOrZero, ig);
emitDispLargeJmp(id, isNew, doffs, asmfm, offset, code, sz, ig);
}
else
{
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/jit/emitarm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ void emitDispAddrRR(regNumber reg1, regNumber reg2, emitAttr attr);
void emitDispAddrRRI(regNumber reg1, regNumber reg2, int imm, emitAttr attr);
void emitDispAddrPUW(regNumber reg, int imm, insOpts opt, emitAttr attr);
void emitDispGC(emitAttr attr);
void emitDispLargeJmp(instrDesc* id,
bool isNew,
bool doffs,
bool asmfm,
unsigned offs = 0,
BYTE* code = 0,
size_t sz = 0,
insGroup* ig = NULL);

void emitDispInsHelp(instrDesc* id,
bool isNew,
Expand Down
155 changes: 141 additions & 14 deletions src/coreclr/jit/emitarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12256,8 +12256,121 @@ void emitter::emitDispInsHex(instrDesc* id, BYTE* code, size_t sz)
}
}

/*****************************************************************************
*
* Handles printing of LARGEJMP pseudo-instruction.
*/

void emitter::emitDispLargeJmp(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* pCode, size_t sz, insGroup* ig)
{
// Note: don't touch the actual instrDesc. If we accidentally messed it up, it would create a very
// difficult-to-find bug.

instrDescJmp idJmp;
instrDescJmp* pidJmp = &idJmp;

memset(&idJmp, 0, sizeof(idJmp));

const instruction ins = id->idIns();
instruction reverseIns;
insFormat reverseFmt;

// Reverse the conditional instruction.
switch (ins)
{
case INS_cbz:
reverseIns = INS_cbnz;
reverseFmt = IF_BI_1A;
break;
case INS_cbnz:
reverseIns = INS_cbz;
reverseFmt = IF_BI_1A;
break;
case INS_tbz:
reverseIns = INS_tbnz;
reverseFmt = IF_BI_1B;
break;
case INS_tbnz:
reverseIns = INS_tbz;
reverseFmt = IF_BI_1B;
break;
default:
reverseIns = emitJumpKindToIns(emitReverseJumpKind(emitInsToJumpKind(ins)));
reverseFmt = IF_BI_0B;
}

pidJmp->idIns(reverseIns);
pidJmp->idInsFmt(reverseFmt);
pidJmp->idOpSize(id->idOpSize());
pidJmp->idAddr()->iiaSetInstrCount(1);
pidJmp->idDebugOnlyInfo(id->idDebugOnlyInfo()); // Share the idDebugOnlyInfo() field.

const size_t bcondSizeOrZero = (pCode == NULL) ? 0 : 4; // Branch is 4 bytes.
emitDispInsHelp(pidJmp, false, doffs, asmfm, offset, pCode, bcondSizeOrZero,
NULL /* force display of pc-relative branch */);

pCode += bcondSizeOrZero;
offset += 4;

// Next, display the unconditional branch.

// Reset the local instrDesc.
memset(&idJmp, 0, sizeof(idJmp));

pidJmp->idIns(INS_b);
pidJmp->idInsFmt(IF_LARGEJMP);

if (id->idIsBound())
{
pidJmp->idSetIsBound();
pidJmp->idAddr()->iiaIGlabel = id->idAddr()->iiaIGlabel;
}
else
{
pidJmp->idAddr()->iiaBBlabel = id->idAddr()->iiaBBlabel;
}

pidJmp->idDebugOnlyInfo(id->idDebugOnlyInfo()); // Share the idDebugOnlyInfo() field.

const size_t brSizeOrZero = (pCode == NULL) ? 0 : 4; // Unconditional branch is 4 bytes.
emitDispInsHelp(pidJmp, isNew, doffs, asmfm, offset, pCode, brSizeOrZero, ig);
}

/*****************************************************************************
*
* Wrapper for emitter::emitDispInsHelp() that handles special large jump
* pseudo-instruction.
*/

void emitter::emitDispIns(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* pCode, size_t sz, insGroup* ig)
{
// Special case: IF_LARGEJMP

if ((id->idInsFmt() == IF_LARGEJMP) && id->idIsBound())
{
// This is a pseudo-instruction format representing a large conditional branch. See the comment
// in emitter::emitOutputLJ() for the full description.
//
// For this pseudo-instruction, we will actually generate:
//
// b<!cond> L_not // 4 bytes. Note that we reverse the condition.
// b L_target // 4 bytes.
// L_not:
//
// These instructions don't exist in the actual instruction stream, so we need to fake them
// up to display them.
emitDispLargeJmp(id, isNew, doffs, asmfm, offset, pCode, sz, ig);
}
else
{
emitDispInsHelp(id, isNew, doffs, asmfm, offset, pCode, sz, ig);
}
}

//--------------------------------------------------------------------
// emitDispIns: Dump the given instruction to jitstdout.
// emitDispInsHelp: Dump the given instruction to jitstdout.
//
// Arguments:
// id - The instruction
Expand All @@ -12272,7 +12385,7 @@ void emitter::emitDispInsHex(instrDesc* id, BYTE* code, size_t sz)
// sz - The size of the instruction, used to display the encoded bytes.
// ig - The instruction group containing the instruction.
//
void emitter::emitDispIns(
void emitter::emitDispInsHelp(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* pCode, size_t sz, insGroup* ig)
{
if (EMITVERBOSE)
Expand All @@ -12284,7 +12397,9 @@ void emitter::emitDispIns(
}

if (pCode == NULL)
{
sz = 0;
}

if (!isNew && !asmfm && sz)
{
Expand Down Expand Up @@ -12394,31 +12509,43 @@ void emitter::emitDispIns(
break;

case IF_BI_1A: // BI_1A ......iiiiiiiiii iiiiiiiiiiittttt Rt simm19:00
case IF_BI_1B: // BI_1B B.......bbbbbiii iiiiiiiiiiittttt Rt imm6, simm14:00
{
assert(insOptsNone(id->idInsOpt()));
emitDispReg(id->idReg1(), size, true);
if (id->idIsBound())

if (fmt == IF_BI_1B)
{
emitPrintLabel(id->idAddr()->iiaIGlabel);
emitDispImm(emitGetInsSC(id), true);
}
else

amanasifkhalid marked this conversation as resolved.
Show resolved Hide resolved
if (id->idAddr()->iiaHasInstrCount())
{
printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
}
break;
int instrCount = id->idAddr()->iiaGetInstrCount();

case IF_BI_1B: // BI_1B B.......bbbbbiii iiiiiiiiiiittttt Rt imm6, simm14:00
assert(insOptsNone(id->idInsOpt()));
emitDispReg(id->idReg1(), size, true);
emitDispImm(emitGetInsSC(id), true);
if (id->idIsBound())
if (ig == nullptr)
{
printf("pc%s%d instructions", (instrCount >= 0) ? "+" : "", instrCount);
}
else
{
unsigned insNum = emitFindInsNum(ig, id);
UNATIVE_OFFSET srcOffs = ig->igOffs + emitFindOffset(ig, insNum + 1);
UNATIVE_OFFSET dstOffs = ig->igOffs + emitFindOffset(ig, insNum + 1 + instrCount);
ssize_t relOffs = (ssize_t)(emitOffsetToPtr(dstOffs) - emitOffsetToPtr(srcOffs));
printf("pc%s%d (%d instructions)", (relOffs >= 0) ? "+" : "", relOffs, instrCount);
}
}
else if (id->idIsBound())
{
emitPrintLabel(id->idAddr()->iiaIGlabel);
}
else
{
printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
}
break;
}
break;

case IF_BR_1A: // BR_1A ................ ......nnnnn..... Rn
assert(insOptsNone(id->idInsOpt()));
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/emitarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ static bool strictArmAsm;

const char* emitVectorRegName(regNumber reg);

void emitDispInsHelp(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* pCode, size_t sz, insGroup* ig);
void emitDispLargeJmp(
instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* pCode, size_t sz, insGroup* ig);
void emitDispInst(instruction ins);
void emitDispImm(ssize_t imm, bool addComma, bool alwaysHex = false);
void emitDispFloatZero();
Expand Down Expand Up @@ -873,6 +877,8 @@ BYTE* emitOutputShortBranch(BYTE* dst, instruction ins, insFormat fmt, ssize_t d
BYTE* emitOutputShortAddress(BYTE* dst, instruction ins, insFormat fmt, ssize_t distVal, regNumber reg);
BYTE* emitOutputShortConstant(
BYTE* dst, instruction ins, insFormat fmt, ssize_t distVal, regNumber reg, emitAttr opSize);
BYTE* emitOutputVectorConstant(
BYTE* dst, ssize_t distVal, regNumber dstReg, regNumber addrReg, emitAttr opSize, emitAttr elemSize);

/*****************************************************************************
*
Expand Down