Fix arm64 apple hfa<float>. #46034
Merged
Fix arm64 apple hfa<float>. #46034
Conversation
This file contains 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
sandreenko
added
arch-arm64
os-mac-os-x
|
/azp list |
This comment has been minimized.
This comment has been minimized.
|
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
sandreenko
force-pushed
the
fixArm64AppleHfa
branch
from
6773295 to
5d4ffe1
Compare
added 4 commits
December 14, 2020 17:43
There was no users that required the exact size, but keeping it required rounding all over the place.
Apple doesn't require struct alignment for it.
sandreenko
force-pushed
the
fixArm64AppleHfa
branch
from
5d4ffe1 to
2d87c52
Compare
This comment has been minimized.
This comment has been minimized.
This was referenced Dec 16, 2020
|
ping @dotnet/jit-contrib |
sdmaclea
approved these changes
Dec 18, 2020
AndyAyersMS
approved these changes
Dec 18, 2020
Comment on lines
+3656
to
+3666
| // TODO-Review: Sometimes we get called on ARM with HFA struct variables that have been promoted, | ||
| // where the struct itself is no longer used because all access is via its member fields. | ||
| // When that happens, the struct is marked as unused and its type has been changed to | ||
| // TYP_INT (to keep the GC tracking code from looking at it). | ||
| // See Compiler::raAssignVars() for details. For example: | ||
| // N002 ( 4, 3) [00EA067C] ------------- return struct $346 | ||
| // N001 ( 3, 2) [00EA0628] ------------- lclVar struct(U) V03 loc2 | ||
| // float V03.f1 (offs=0x00) -> V12 tmp7 | ||
| // f8 (last use) (last use) $345 | ||
| // Here, the "struct(U)" shows that the "V03 loc2" variable is unused. Not shown is that V03 | ||
| // is now TYP_INT in the local variable table. It's not really unused, because it's in the tree. |
There was a problem hiding this comment.
Realize this is a pre-existing comment, but raAssignVars is long gone....
There was a problem hiding this comment.
Thank you, I will fix the comment in the next pr.
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Apple Arm64 does not require struct alignment for HFA, instead, it uses 4-byte stack alignment for this type. Add Jit support for this. Closes #45780.
The changes:
0296b35: A small fix in dump printing.
ac6fada: Keep the aligned size in
fgArgInfo.There were no users that required the exact size, but keeping it required roundings all over the place.
So now both
fgArgInfoandGenTreePutArgkeep the align size (with padding after the arg value), but the alignment is different on different platforms.ee4076f: Fix float HFA stack passing.
Apple doesn't require struct alignment for it.
The main change is introducing
eeGetArgAlignmentthat returns alignment for an argument type. I am planning to extend this method soon (see #46026).The second change is fix for
LclVarDsc::lvSize()so it returns12for such variables.Also, I am trying to use less
#if defined(OSX_ARM64_ABI)to make #45501 easier.2d87c52: Fix
lvaMapSimd12ToSimd16for arm64 apple arguments.Now when
lvSizereturns12forSIMD12arguments on arm64 apple we need to fix some asserts that do not expect it.Fixes 46 tests (including pri1 tests):
Note: it does not fully fix
because they are using reflection and additional VM fixes are needed.