Skip to content

Add disasm comments for field data addresses and code addresses #70437

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

Add disasm comments for field data addresses and code addresses #70437

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
61f464c
Add disasm comments for field data addresses and code addresses
jakobbotsch Jun 8, 2022
35d57b0
Remove unnecessary ifdef
jakobbotsch Jun 8, 2022
ceaaed1
Fix impTokenToHandle when importing parent
jakobbotsch Jun 8, 2022
a95d33e
Do not lie about critical sections being method handles
jakobbotsch Jun 8, 2022
0ec8ac2
Change "code address" -> "code"
jakobbotsch Jun 8, 2022
5122904
Merge branch 'main' of github.com:dotnet/runtime into disasm-handle-c…
jakobbotsch Jun 9, 2022
932d5a5
Remove accidentally added file
jakobbotsch Jun 9, 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
13 changes: 7 additions & 6 deletions src/coreclr/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2178,7 +2178,7 @@ void CodeGen::instGen_Set_Reg_To_Imm(emitAttr size,
{
if (emitter::emitIns_valid_imm_for_mov(imm, size))
{
GetEmitter()->emitIns_R_I(INS_mov, size, reg, imm);
GetEmitter()->emitIns_R_I(INS_mov, size, reg, imm, INS_OPTS_NONE DEBUGARG(targetHandle) DEBUGARG(gtFlags));
}
else
{
Expand Down Expand Up @@ -2224,7 +2224,9 @@ void CodeGen::instGen_Set_Reg_To_Imm(emitAttr size,
imm16 = ~imm16;
}

GetEmitter()->emitIns_R_I_I(ins, size, reg, imm16, i, INS_OPTS_LSL);
GetEmitter()->emitIns_R_I_I(ins, size, reg, imm16, i,
INS_OPTS_LSL DEBUGARG(i == 0 ? targetHandle : 0)
DEBUGARG(i == 0 ? gtFlags : GTF_EMPTY));

// Once the initial movz/movn is emitted the remaining instructions will all use movk
ins = INS_movk;
Expand Down Expand Up @@ -2258,8 +2260,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
{
case GT_CNS_INT:
{
GenTreeIntConCommon* con = tree->AsIntConCommon();
ssize_t cnsVal = con->IconValue();
GenTreeIntCon* con = tree->AsIntCon();
ssize_t cnsVal = con->IconValue();

emitAttr attr = emitActualTypeSize(targetType);
// TODO-CQ: Currently we cannot do this for all handles because of
Expand All @@ -2275,8 +2277,7 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
}

instGen_Set_Reg_To_Imm(attr, targetReg, cnsVal,
INS_FLAGS_DONT_CARE DEBUGARG(tree->AsIntCon()->gtTargetHandle)
DEBUGARG(tree->AsIntCon()->gtFlags));
INS_FLAGS_DONT_CARE DEBUGARG(con->gtTargetHandle) DEBUGARG(con->gtFlags));
regSet.verifyRegUsed(targetReg);
}
break;
Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/jit/codegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ void CodeGen::instGen_Set_Reg_To_Imm(emitAttr size,
}
else
{
GetEmitter()->emitIns_R_I(INS_mov, size, reg, imm DEBUGARG(gtFlags));
GetEmitter()->emitIns_R_I(INS_mov, size, reg, imm DEBUGARG(targetHandle) DEBUGARG(gtFlags));
}
}
regSet.verifyRegUsed(reg);
Expand All @@ -462,8 +462,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
{
// relocatable values tend to come down as a CNS_INT of native int type
// so the line between these two opcodes is kind of blurry
GenTreeIntConCommon* con = tree->AsIntConCommon();
ssize_t cnsVal = con->IconValue();
GenTreeIntCon* con = tree->AsIntCon();
ssize_t cnsVal = con->IconValue();

emitAttr attr = emitActualTypeSize(targetType);
// Currently this cannot be done for all handles due to
Expand All @@ -482,7 +482,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
attr = EA_SET_FLG(attr, EA_BYREF_FLG);
}

instGen_Set_Reg_To_Imm(attr, targetReg, cnsVal, INS_FLAGS_DONT_CARE DEBUGARG(0) DEBUGARG(tree->gtFlags));
instGen_Set_Reg_To_Imm(attr, targetReg, cnsVal,
INS_FLAGS_DONT_CARE DEBUGARG(con->gtTargetHandle) DEBUGARG(con->gtFlags));
regSet.verifyRegUsed(targetReg);
}
break;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2269,7 +2269,7 @@ class Compiler

GenTree* gtNewIndOfIconHandleNode(var_types indType, size_t value, GenTreeFlags iconFlags, bool isInvariant);

GenTree* gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields = nullptr);
GenTreeIntCon* gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields = nullptr);

GenTreeFlags gtTokenToIconFlags(unsigned token);

Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,9 +902,8 @@ inline GenTree* Compiler::gtNewLargeOperNode(genTreeOps oper, var_types type, Ge
* that may need to be fixed up).
*/

inline GenTree* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields)
inline GenTreeIntCon* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags, FieldSeqNode* fields)
{
GenTree* node;
assert((flags & (GTF_ICON_HDL_MASK | GTF_ICON_FIELD_OFF)) != 0);

// Interpret "fields == NULL" as "not a field."
Expand All @@ -913,6 +912,7 @@ inline GenTree* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags flags,
fields = FieldSeqStore::NotAField();
}

GenTreeIntCon* node;
#if defined(LATE_DISASM)
node = new (this, LargeOpOpcode()) GenTreeIntCon(TYP_I_IMPL, value, fields DEBUGARG(/*largeNode*/ true));
#else
Expand Down Expand Up @@ -1370,6 +1370,7 @@ inline void GenTree::SetOper(genTreeOps oper, ValueNumberUpdate vnUpdate)
{
case GT_CNS_INT:
AsIntCon()->gtFieldSeq = FieldSeqStore::NotAField();
INDEBUG(AsIntCon()->gtTargetHandle = 0);
break;
#if defined(TARGET_ARM)
case GT_MUL_LONG:
Expand Down
44 changes: 30 additions & 14 deletions src/coreclr/jit/emit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4045,25 +4045,48 @@ void emitter::emitRecomputeIGoffsets()
//
// Arguments:
// handle - a constant value to display a comment for
// cookie - the cookie stored with the handle
// flags - a flag that the describes the handle
//
void emitter::emitDispCommentForHandle(size_t handle, GenTreeFlags flag)
void emitter::emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlags flag)
{
#ifdef DEBUG
if (handle == 0)
{
return;
}

#ifdef TARGET_XARCH
const char* commentPrefix = " ;";
#else
const char* commentPrefix = " //";
#endif

flag &= GTF_ICON_HDL_MASK;
const char* str = nullptr;

if (cookie != 0)
{
if (flag == GTF_ICON_FTN_ADDR)
{
const char* className = nullptr;
const char* methName =
emitComp->eeGetMethodName(reinterpret_cast<CORINFO_METHOD_HANDLE>(cookie), &className);
printf("%s code for %s:%s", commentPrefix, className, methName);
return;
}

if ((flag == GTF_ICON_STATIC_HDL) || (flag == GTF_ICON_STATIC_BOX_PTR))
{
const char* className = nullptr;
const char* fieldName =
emitComp->eeGetFieldName(reinterpret_cast<CORINFO_FIELD_HANDLE>(cookie), &className);
printf("%s %s for %s%s%s", commentPrefix, flag == GTF_ICON_STATIC_HDL ? "data" : "box", className,
className != nullptr ? ":" : "", fieldName);
return;
}
}

if (handle == 0)
{
return;
}

const char* str = nullptr;
if (flag == GTF_ICON_STR_HDL)
{
const WCHAR* wstr = emitComp->eeGetCPString(handle);
Expand Down Expand Up @@ -4103,8 +4126,6 @@ void emitter::emitDispCommentForHandle(size_t handle, GenTreeFlags flag)
{
str = emitComp->eeGetClassName(reinterpret_cast<CORINFO_CLASS_HANDLE>(handle));
}
#ifndef TARGET_XARCH
// These are less useful for xarch:
else if (flag == GTF_ICON_CONST_PTR)
{
str = "const ptr";
Expand Down Expand Up @@ -4133,11 +4154,6 @@ void emitter::emitDispCommentForHandle(size_t handle, GenTreeFlags flag)
{
str = "token handle";
}
else
{
str = "unknown";
}
#endif // TARGET_XARCH

if (str != nullptr)
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/emit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ class emitter

void emitRecomputeIGoffsets();

void emitDispCommentForHandle(size_t handle, GenTreeFlags flags);
void emitDispCommentForHandle(size_t handle, size_t cookie, GenTreeFlags flags);

/************************************************************************/
/* The following describes a single instruction */
Expand Down Expand Up @@ -554,7 +554,7 @@ class emitter

#endif // TARGET_XARCH

#ifdef DEBUG // This information is used in DEBUG builds to display the method name for call instructions
#ifdef DEBUG // This information is used in DEBUG builds for additional diagnostics

struct instrDesc;

Expand Down
29 changes: 24 additions & 5 deletions src/coreclr/jit/emitarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3740,7 +3740,8 @@ void emitter::emitIns_R_I(instruction ins,
emitAttr attr,
regNumber reg,
ssize_t imm,
insOpts opt /* = INS_OPTS_NONE */ DEBUGARG(GenTreeFlags gtFlags))
insOpts opt /* = INS_OPTS_NONE */
DEBUGARG(size_t targetHandle /* = 0 */) DEBUGARG(GenTreeFlags gtFlags /* = GTF_EMPTY */))
{
emitAttr size = EA_SIZE(attr);
emitAttr elemsize = EA_UNKNOWN;
Expand Down Expand Up @@ -3990,7 +3991,11 @@ void emitter::emitIns_R_I(instruction ins,
id->idInsOpt(opt);

id->idReg1(reg);
INDEBUG(id->idDebugOnlyInfo()->idFlags = gtFlags);

#ifdef DEBUG
id->idDebugOnlyInfo()->idMemCookie = targetHandle;
id->idDebugOnlyInfo()->idFlags = gtFlags;
#endif

dispIns(id);
appendToCurIG(id);
Expand Down Expand Up @@ -4927,8 +4932,13 @@ void emitter::emitIns_R_R(
* Add an instruction referencing a register and two constants.
*/

void emitter::emitIns_R_I_I(
instruction ins, emitAttr attr, regNumber reg, ssize_t imm1, ssize_t imm2, insOpts opt /* = INS_OPTS_NONE */)
void emitter::emitIns_R_I_I(instruction ins,
emitAttr attr,
regNumber reg,
ssize_t imm1,
ssize_t imm2,
insOpts opt /* = INS_OPTS_NONE */
DEBUGARG(size_t targetHandle /* = 0 */) DEBUGARG(GenTreeFlags gtFlags /* = 0 */))
{
emitAttr size = EA_SIZE(attr);
insFormat fmt = IF_NONE;
Expand Down Expand Up @@ -5015,6 +5025,11 @@ void emitter::emitIns_R_I_I(

id->idReg1(reg);

#ifdef DEBUG
id->idDebugOnlyInfo()->idFlags = gtFlags;
id->idDebugOnlyInfo()->idMemCookie = targetHandle;
#endif

dispIns(id);
appendToCurIG(id);
}
Expand Down Expand Up @@ -12487,7 +12502,7 @@ void emitter::emitDispIns(
}
else
{
emitDispCommentForHandle(id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
emitDispCommentForHandle(id->idDebugOnlyInfo()->idMemCookie, 0, id->idDebugOnlyInfo()->idFlags);
}
break;

Expand Down Expand Up @@ -12623,6 +12638,7 @@ void emitter::emitDispIns(
case IF_DI_1A: // DI_1A X.......shiiiiii iiiiiinnnnn..... Rn imm(i12,sh)
emitDispReg(id->idReg1(), size, true);
emitDispImmOptsLSL12(emitGetInsSC(id), id->idInsOpt());
emitDispCommentForHandle(0, id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
break;

case IF_DI_1B: // DI_1B X........hwiiiii iiiiiiiiiiiddddd Rd imm(i16,hw)
Expand All @@ -12641,18 +12657,21 @@ void emitter::emitDispIns(
emitDispImm(hwi.immHW * 16, false);
}
}
emitDispCommentForHandle(0, id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
break;

case IF_DI_1C: // DI_1C X........Nrrrrrr ssssssnnnnn..... Rn imm(N,r,s)
emitDispReg(id->idReg1(), size, true);
bmi.immNRS = (unsigned)emitGetInsSC(id);
emitDispImm(emitDecodeBitMaskImm(bmi, size), false);
emitDispCommentForHandle(0, id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
break;

case IF_DI_1D: // DI_1D X........Nrrrrrr ssssss.....ddddd Rd imm(N,r,s)
emitDispReg(encodingZRtoSP(id->idReg1()), size, true);
bmi.immNRS = (unsigned)emitGetInsSC(id);
emitDispImm(emitDecodeBitMaskImm(bmi, size), false);
emitDispCommentForHandle(0, id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
break;

case IF_DI_2A: // DI_2A X.......shiiiiii iiiiiinnnnnddddd Rd Rn imm(i12,sh)
Expand Down
12 changes: 9 additions & 3 deletions src/coreclr/jit/emitarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,8 @@ void emitIns_R_I(instruction ins,
emitAttr attr,
regNumber reg,
ssize_t imm,
insOpts opt = INS_OPTS_NONE DEBUGARG(GenTreeFlags gtFlags = GTF_EMPTY));
insOpts opt = INS_OPTS_NONE DEBUGARG(size_t targetHandle = 0)
DEBUGARG(GenTreeFlags gtFlags = GTF_EMPTY));

void emitIns_R_F(instruction ins, emitAttr attr, regNumber reg, double immDbl, insOpts opt = INS_OPTS_NONE);

Expand All @@ -740,8 +741,13 @@ void emitIns_R_R(instruction ins, emitAttr attr, regNumber reg1, regNumber reg2,
emitIns_R_R(ins, attr, reg1, reg2);
}

void emitIns_R_I_I(
instruction ins, emitAttr attr, regNumber reg1, ssize_t imm1, ssize_t imm2, insOpts opt = INS_OPTS_NONE);
void emitIns_R_I_I(instruction ins,
emitAttr attr,
regNumber reg1,
ssize_t imm1,
ssize_t imm2,
insOpts opt = INS_OPTS_NONE DEBUGARG(size_t targetHandle = 0)
DEBUGARG(GenTreeFlags gtFlags = GTF_EMPTY));

void emitIns_R_R_I(
instruction ins, emitAttr attr, regNumber reg1, regNumber reg2, ssize_t imm, insOpts opt = INS_OPTS_NONE);
Expand Down
29 changes: 26 additions & 3 deletions src/coreclr/jit/emitxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3219,6 +3219,11 @@ void emitter::emitHandleMemOp(GenTreeIndir* indir, instrDesc* id, insFormat fmt,

id->idAddr()->iiaFieldHnd = fldHnd;
id->idInsFmt(emitMapFmtForIns(emitMapFmtAtoM(fmt), ins));

#ifdef DEBUG
id->idDebugOnlyInfo()->idFlags = GTF_ICON_STATIC_HDL;
id->idDebugOnlyInfo()->idMemCookie = reinterpret_cast<size_t>(fldHnd);
#endif
}
else if ((memBase != nullptr) && memBase->IsCnsIntOrI() && memBase->isContained())
{
Expand Down Expand Up @@ -3279,6 +3284,14 @@ void emitter::emitHandleMemOp(GenTreeIndir* indir, instrDesc* id, insFormat fmt,
// disp must have already been set in the instrDesc constructor.
assert(emitGetInsAmdAny(id) == indir->Offset()); // make sure "disp" is stored properly
}

#ifdef DEBUG
if ((memBase != nullptr) && memBase->IsIconHandle() && memBase->isContained())
{
id->idDebugOnlyInfo()->idFlags = memBase->gtFlags;
id->idDebugOnlyInfo()->idMemCookie = memBase->AsIntCon()->gtTargetHandle;
}
#endif
}

// Takes care of storing all incoming register parameters
Expand Down Expand Up @@ -4126,7 +4139,10 @@ void emitter::emitIns_R(instruction ins, emitAttr attr, regNumber reg)
* Add an instruction referencing a register and a constant.
*/

void emitter::emitIns_R_I(instruction ins, emitAttr attr, regNumber reg, ssize_t val DEBUGARG(GenTreeFlags gtFlags))
void emitter::emitIns_R_I(instruction ins,
emitAttr attr,
regNumber reg,
ssize_t val DEBUGARG(size_t targetHandle) DEBUGARG(GenTreeFlags gtFlags))
{
emitAttr size = EA_SIZE(attr);

Expand Down Expand Up @@ -4259,7 +4275,11 @@ void emitter::emitIns_R_I(instruction ins, emitAttr attr, regNumber reg, ssize_t
id->idInsFmt(fmt);
id->idReg1(reg);
id->idCodeSize(sz);
INDEBUG(id->idDebugOnlyInfo()->idFlags = gtFlags);

#ifdef DEBUG
id->idDebugOnlyInfo()->idFlags = gtFlags;
id->idDebugOnlyInfo()->idMemCookie = targetHandle;
#endif

dispIns(id);
emitCurIGsize += sz;
Expand Down Expand Up @@ -9118,7 +9138,7 @@ void emitter::emitDispIns(
{ // (val < 0)
printf("-0x%IX", -val);
}
emitDispCommentForHandle(srcVal, id->idDebugOnlyInfo()->idFlags);
emitDispCommentForHandle(srcVal, id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
}
break;

Expand Down Expand Up @@ -9189,6 +9209,9 @@ void emitter::emitDispIns(
}
printf("%s, %s", emitRegName(id->idReg1(), attr), sstr);
emitDispAddrMode(id);

emitDispCommentForHandle(emitGetInsAmdAny(id), id->idDebugOnlyInfo()->idMemCookie,
id->idDebugOnlyInfo()->idFlags);
break;

case IF_RRW_ARD_CNS:
Expand Down
Loading