-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix stress apple arm64 assertion '(thisFieldOffset + EA_SIZE_IN_BYTES(attr)) <= areaSize' #48936
Conversation
/azp run runtime-coreclr jitstress |
Azure Pipelines successfully started running 1 pipeline(s). |
PTAL @dotnet/jit-contrib |
#endif // FEATURE_SIMD | ||
{ | ||
emitAttr attr = emitTypeSize(type); | ||
GetEmitter()->emitIns_S_R(ins_Store(type), attr, reg, outArgVarNum, thisFieldOffset); |
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.
@sandreenko Correct me if I am wrong - on non macos-arm64 TYP_SIMD12
is going to take 16 bytes on the stack and the JIT uses str Qd, [Xn]
instruction
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.
@echesakovMSFT there are different cases:
- pass in regs;
- pass on stack;
x - LCL_VAR
- LCL_FLD
- OBJ/IND
- FIELD_LIST
usually, if we pass LCL_VAR SIMD12
we will use str Qd, [Xn]
, because we know that LCL_VAR has 4 bytes after 12 bytes of SIMD that we can read, but if SIMD12 is a field of a struct then we will have to read exactly 12 bytes, so we will have IND SIMD12(ADDR(LCL_FLD))
and it has similar logic.
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.
Got it, thanks!
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.
LGTM
#endif // FEATURE_SIMD | ||
{ | ||
emitAttr attr = emitTypeSize(type); | ||
GetEmitter()->emitIns_S_R(ins_Store(type), attr, reg, outArgVarNum, thisFieldOffset); |
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.
Got it, thanks!
src/coreclr/jit/emitarm64.h
Outdated
void emitStoreSIMD12ToLclOffset(unsigned varNum, unsigned offset, regNumber opReg, regNumber tmpReg) | ||
{ | ||
assert(varNum != BAD_VAR_NUM); | ||
assert((opReg != REG_NA) && (tmpReg != REG_NA)); |
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.
Since you validating incoming opReg
ad tmpReg
you also check here that they have right types, i.e. opReg
is SIMD, and tmpReg
is a general purpose register.
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.
I have added assert for opReg is SIMD
but for tmp
the type is unknown: for x86 we require an xmm register, for other it looks random, some places allocate integer, the other xmm.
2f25add
to
83d5e06
Compare
Fixes #48787