Fix arm64 wrong multireg copies from reg to small type field in memory #46345
Conversation
sandreenko
added
the
area-CodeGen-coreclr
|
Can anybody approve this? |
sandreenko
force-pushed
the
fixArm64MultiReg
branch
from
ff52e75
to
efe6df2
Compare
|
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
sandreenko
force-pushed
the
fixArm64MultiReg
branch
from
efe6df2
to
6ff46b8
Compare
|
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
@AndyAyersMS I've changed the fix, please take another look. cc @dotnet/jit-contrib , @jkoritzinsky |
src/coreclr/jit/instr.cpp
Outdated
| // Notes: | ||
| // The fuctiuon currently does not expect float srcReg with integral dstType and will assert on such cases. | ||
| // | ||
| instruction CodeGenInterface::ins_Store(regNumber srcReg, var_types dstType, bool aligned /*=false*/) |
There was a problem hiding this comment.
Would prefer not to see overloads like this with optional parameters. Can we rename one or the other?
There was a problem hiding this comment.
Also not clear to me why we don't just force all stores to use the version that does more checking...
There was a problem hiding this comment.
Makes sense, I am thinking how to rename one of them now.
Also not clear to me why we don't just force all stores to use the version that does more checking...
It is possible but requires more refactoring changes, for example, emitInsStoreLcl and emitInsStoreInd should just receive GenTree* and calculate instruction ins, emitAttr attr inside. I will try to find time for this change later as a separate PR.
|
did this fix #46278 ? |
Yes, thanks for pointing this, I forgot to link that issue. |
ghost
locked as resolved and limited conversation to collaborators
Closes #46240