-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
Return accurate size of addressing mode dsp/dsp+cnst instr descriptor #47284
Conversation
@dotnet/jit-contrib |
@kunalspathak Have you confirmed our hypothesis that size of the instructions descriptors are the same with crossgen (x86) as we discussed yesterday? |
@@ -7440,6 +7438,30 @@ size_t emitter::emitSizeOfInsDsc(instrDesc* id) | |||
return sizeof(instrDesc); | |||
} | |||
} | |||
case ID_OP_AMD: | |||
case ID_OP_AMD_CNS: | |||
if (id->idIsLargeCns()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on emitNewInstrAmdCns
, I would expect it to be:
if (id->idIsLargeCns())
{
if (id->idIsLargeDsp())
{
return sizeof(instrDescCnsAmd);
}
else
{
return sizeof(instrDescCns);
}
}
else
{
if (id->idIsLargeDsp())
{
return sizeof(instrDescAmd);
}
else
{
return sizeof(instrDesc);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually yes.
Yes @echesakovMSFT - they are same. I also crossgen stind_i1.ilproj project which reproed the issue and used |
During spmi
asmdiffs
with cross compiled jit, I noticed we hit the following assertruntime/src/coreclr/jit/emit.cpp
Line 1270 in 035821a
It happens because for
ID_OP_AMD
andID_OP_AMD_CNS
format, we createinstrDescCnsAmd
but then inemitSizeOfInsDsc()
, when we try to calculate the size, we calculate the size ofinstrDescCnsDsp
instead. They don't match for cross compilation scenario like runningsuperpmi asmdiffs
for cross target. E.g. While running withclrjit_win_x86_x64.dll
,sizeof(instrDescCnsAmd) == 40
butsizeof(instrDescCnsDsp) == 32
. The is becauseinstrDescCnsDsp
contains a field of typetarget_ssize_t
which is different for x64 vs. x86.runtime/src/coreclr/jit/emit.h
Line 1401 in 035821a
The fix is to return the size of right struct for these formats.