Return accurate size of addressing mode dsp/dsp+cnst instr descriptor#47284
Merged
kunalspathak merged 2 commits intodotnet:masterfrom Jan 25, 2021
Merged
Return accurate size of addressing mode dsp/dsp+cnst instr descriptor#47284kunalspathak merged 2 commits intodotnet:masterfrom
kunalspathak merged 2 commits intodotnet:masterfrom
Conversation
Contributor
Author
|
@dotnet/jit-contrib |
Contributor
|
@kunalspathak Have you confirmed our hypothesis that size of the instructions descriptors are the same with crossgen (x86) as we discussed yesterday? |
BruceForstall
suggested changes
Jan 23, 2021
| } | ||
| case ID_OP_AMD: | ||
| case ID_OP_AMD_CNS: | ||
| if (id->idIsLargeCns()) |
Contributor
There was a problem hiding this comment.
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);
}
}
Contributor
Author
Yes @echesakovMSFT - they are same. I also crossgen stind_i1.ilproj project which reproed the issue and used |
BruceForstall
approved these changes
Jan 25, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During spmi
asmdiffswith 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_AMDandID_OP_AMD_CNSformat, we createinstrDescCnsAmdbut then inemitSizeOfInsDsc(), when we try to calculate the size, we calculate the size ofinstrDescCnsDspinstead. They don't match for cross compilation scenario like runningsuperpmi asmdiffsfor cross target. E.g. While running withclrjit_win_x86_x64.dll,sizeof(instrDescCnsAmd) == 40butsizeof(instrDescCnsDsp) == 32. The is becauseinstrDescCnsDspcontains a field of typetarget_ssize_twhich 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.