Skip to content
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

osx-arm64 ABI changes. #41130

Closed
2 of 3 tasks
sdmaclea opened this issue Aug 21, 2020 · 15 comments
Closed
2 of 3 tasks

osx-arm64 ABI changes. #41130

sdmaclea opened this issue Aug 21, 2020 · 15 comments
Assignees
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Bottom Up Work Not part of a theme, epic, or user story os-macos-bigsur (macOS11) User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@sdmaclea
Copy link
Contributor

sdmaclea commented Aug 21, 2020

osx-arm64 has a slightly different calling convention than the arm specified conventions

The obvious things that stood out to me was

  • R18/X18/W18 reserved
  • Removal of packing restrictions on arguments passed on the stack

Tasks from @sandreenko:

  • Support small stack arguments passing in managed <-> native calls;

  • Support small stack arguments passing via reflection (VM changes);

  • Support 16-byte struct passing starting with even register (like x1,x2);

category:correctness
theme:calling-convention
skill-level:expert
cost:large

@sdmaclea sdmaclea added arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-macos-bigsur (macOS11) labels Aug 21, 2020
@sdmaclea sdmaclea added this to the 6.0.0 milestone Aug 21, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 21, 2020
@sdmaclea
Copy link
Contributor Author

I did a quick survey of R18 usage. It looks like it is not used in any of our manually written assembly. It also looks like the JIT doesn't generate code using that register (perhaps because Windows-Arm64 reserves it).

@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Aug 24, 2020
@sandreenko
Copy link
Contributor

sandreenko@9abb9ad reproduces the issue:

Error in Function Marshal_InMany(Native Client)
Expected: 1, 2, 3, 4, 5, 6, 7, 8, 9, 10
Actual:1, 2, 3, 4, 5, 6, 7, 8, 9, 0
InMany return value is wrong

working on a fix.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Sep 2, 2020

These failing tests are also related to this issue

// Assertion failed: (false && "null argument."), function SystemNative_ForkAndExecProcess, 
// file /Users/steve/git/runtime/src/libraries/Native/Unix/System.Native/pal_process.c, line 246.
baseservices/exceptions/stackoverflow/stackoverflowtester/stackoverflowtester.sh
Loader/binding/tracing/BinderTracingTest.ResolutionFlow/BinderTracingTest.ResolutionFlow.sh
Loader/binding/tracing/BinderTracingTest.Basic/BinderTracingTest.Basic.sh
profiler/unittest/metadatagetdispenser/metadatagetdispenser.sh
profiler/unittest/getappdomainstaticaddress/getappdomainstaticaddress.sh
profiler/eventpipe/eventpipe/eventpipe.sh
profiler/eventpipe/eventpipe_readevents/eventpipe_readevents.sh
profiler/gc/gc/gc.sh
profiler/gc/gcbasic/gcbasic.sh
profiler/elt/slowpatheltenter/slowpatheltenter.sh
profiler/elt/slowpatheltleave/slowpatheltleave.sh
ilasm/PortablePdb/IlasmPortablePdbTests/IlasmPortablePdbTests.sh
ilasm/System/Runtime/CompilerServices/MethodImplOptionsTests/MethodImplOptionsTests.sh

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Sep 3, 2020

These failures are also related

JIT/SIMD/Vector3Interop_r/Vector3Interop_r.sh
JIT/SIMD/Vector3Interop_ro/Vector3Interop_ro.sh

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Sep 6, 2020

These p1 interop test failures are also related

JIT/jit64/hfa/main/testG/hfa_nd2G_r/hfa_nd2G_r.sh
JIT/jit64/hfa/main/testG/hfa_sd2G_d/hfa_sd2G_d.sh
JIT/jit64/hfa/main/testG/hfa_sf2G_d/hfa_sf2G_d.sh
JIT/jit64/hfa/main/testG/hfa_nf2G_r/hfa_nf2G_r.sh
JIT/jit64/hfa/main/testG/hfa_sd2G_r/hfa_sd2G_r.sh
JIT/jit64/hfa/main/testG/hfa_nd2G_d/hfa_nd2G_d.sh
JIT/jit64/hfa/main/testG/hfa_nf2G_d/hfa_nf2G_d.sh
JIT/jit64/hfa/main/testG/hfa_sf2G_r/hfa_sf2G_r.sh
JIT/jit64/hfa/main/testA/hfa_nf2A_d/hfa_nf2A_d.sh
JIT/jit64/hfa/main/testA/hfa_sf2A_r/hfa_sf2A_r.sh
JIT/jit64/hfa/main/testA/hfa_sf2A_d/hfa_sf2A_d.sh
JIT/jit64/hfa/main/testA/hfa_nf2A_r/hfa_nf2A_r.sh
JIT/jit64/hfa/main/testC/hfa_nf2C_r/hfa_nf2C_r.sh
JIT/jit64/hfa/main/testC/hfa_sf2C_d/hfa_sf2C_d.sh
JIT/jit64/hfa/main/testC/hfa_sf2C_r/hfa_sf2C_r.sh
JIT/jit64/hfa/main/testC/hfa_nf2C_d/hfa_nf2C_d.sh
JIT/jit64/hfa/main/testB/hfa_nf2B_d/hfa_nf2B_d.sh
JIT/jit64/hfa/main/testB/hfa_sf2B_r/hfa_sf2B_r.sh
JIT/jit64/hfa/main/testB/hfa_sf2B_d/hfa_sf2B_d.sh
JIT/jit64/hfa/main/testB/hfa_nf2B_r/hfa_nf2B_r.sh

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Sep 8, 2020

This is also related

Interop/PInvoke/Generics/GenericsTest/GenericsTest.sh

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Sep 9, 2020

This is also related... seg faults calling SystemNative_ForkAndExecProcess

JIT/superpmi/superpmicollect/superpmicollect.sh

@JulieLeeMSFT JulieLeeMSFT added the Epic Groups multiple user stories. Can be grouped under a theme. label Oct 12, 2020
@sandreenko
Copy link
Contributor

A small update before my vacation: master...sandreenko:Arm64OSX is a combination of Jit changes that are on reviews right now and initial VM work. It passes tests where we don't need marshaling (when we call only with primitive types, no pointers or structs) but fails with marshaling because VM keeps calculating sizes in stack slots and corrupts arguments.

To see the issue run lldb -- Tests/Core_Root/corerun Interop/PInvoke/Generics/GenericsTest/GenericsTest.dll with export COMPlus_JitHashDump=7798599a (ILStubClass:IL_STUB_PInvoke(bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,long) (MethodHash=7798599a)) and export COMPlus_JitHalt=TestVector128B stop at the call to IL_STUB_PInvoke, you will see that arguments are passed correctly:

->  0x2819c6848: bl     0x281785068
    0x2819c684c: nop
    0x2819c6850: mov    x17, #0x198
    0x2819c6854: ldr    q16, [x29, x17]
Target 0: (corerun) stopped.
(lldb) memory read -s1 -fu -c20 $sp
0x16fdfe410: 1 <- true bool
0x16fdfe411: 0 <- false bool
0x16fdfe412: 1 <- true bool etc.
0x16fdfe413: 0 
0x16fdfe414: 1
0x16fdfe415: 0
0x16fdfe416: 1
0x16fdfe417: 0
0x16fdfe418: 184 <- our pointer1 
0x16fdfe419: 229 <- our pointer2
0x16fdfe41a: 223 <- our pointer3
0x16fdfe41b: 111 <- out pointer4

but then ILStubClass:IL_STUB_PInvoke(bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,bool,long) mixes them and does:


               [000144] --CXG-------              *  CALL ind unman void  
               [000118] ------------ arg0         +--*  LCL_VAR   int    V18 loc1         
               [000119] ------------ arg1         +--*  LCL_VAR   int    V19 loc2         
               [000120] ------------ arg2         +--*  LCL_VAR   int    V20 loc3         
               [000121] ------------ arg3         +--*  LCL_VAR   int    V21 loc4         
               [000122] ------------ arg4         +--*  LCL_VAR   int    V22 loc5         
               [000123] ------------ arg5         +--*  LCL_VAR   int    V23 loc6         
               [000124] ------------ arg6         +--*  LCL_VAR   int    V24 loc7         
               [000125] ------------ arg7         +--*  LCL_VAR   int    V25 loc8         
               [000126] ------------ arg8         +--*  LCL_VAR   int    V26 loc9         
               [000127] ------------ arg9         +--*  LCL_VAR   int    V27 loc10        
               [000128] ------------ arg10        +--*  LCL_VAR   int    V28 loc11        
               [000129] ------------ arg11        +--*  LCL_VAR   int    V29 loc12        
               [000130] ------------ arg12        +--*  LCL_VAR   int    V30 loc13        
               [000131] ------------ arg13        +--*  LCL_VAR   int    V31 loc14        
               [000132] ------------ arg14        +--*  LCL_VAR   int    V32 loc15        
               [000133] ------------ arg15        +--*  LCL_VAR   int    V33 loc16        
               [000134] ------------ arg16        +--*  LCL_VAR   long   V34 loc17        
               [000143] ------------ calli tgt    \--*  LCL_VAR   long   V37 tmp2 

when instead of int for the first 17 arguments we should see byte, the type is returned from info.compCompHnd->getArgType(sig, sigArgs, &classHnd).

The full log for the problem stub is attached.
wrongStubCompilation.arm64.osx.txt

@JulieLeeMSFT JulieLeeMSFT added Team Epic and removed Epic Groups multiple user stories. Can be grouped under a theme. labels Oct 19, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@JulieLeeMSFT JulieLeeMSFT removed the JitUntriaged CLR JIT issues needing additional triage label Nov 2, 2020
@JulieLeeMSFT JulieLeeMSFT added User Story A single user-facing feature. Can be grouped under an epic. Bottom Up Work Not part of a theme, epic, or user story and removed Team Epic labels Nov 16, 2020
@sdmaclea
Copy link
Contributor Author

@sandreenko Is the Arm64OSX branch representative of your current status. I would like to pull your changes into the SDK and see if it fixes any of the issues we are seeing.

@sandreenko
Copy link
Contributor

@sandreenko Is the Arm64OSX branch representative of your current status. I would like to pull your changes into the SDK and see if it fixes any of the issues we are seeing.

I am working in the same branch master...sandreenko:Arm64OSX, it passes tests with small arguments, like

ilasm/System/Runtime/CompilerServices/MethodImplOptionsTests/MethodImplOptionsTests.dll
Loader/binding/tracing/BinderTracingTest.Basic/BinderTracingTest.Basic.dll

and profiler tests:

/Users/seandree/git/runtime/artifacts/tests/coreclr/OSX.arm64.Debug/Tests/Core_Root/corerun  /Users/seandree/git/runtime/artifacts/tests/coreclr/OSX.arm64.Debug/profiler/unittest/metadatagetdispenser/metadatagetdispenser.dll

Profiler path: /Users/seandree/git/runtime/artifacts/tests/coreclr/OSX.arm64.Debug/profiler/unittest/metadatagetdispenser/libProfiler.dylib
Profiler.dll!DllGetClassObject
Profiler.dll!Profiler::Initialize
Initialize started
Got IMetaDataDispenserEx
ModuleLoadStarted exiting
Got IMetaDataDispenserEx
ModuleLoadStarted exiting
Got IMetaDataDispenserEx
ModuleLoadStarted exiting
Got IMetaDataDispenserEx
ModuleLoadStarted exiting
Profiler.dll!Profiler::Shutdown
PROFILER TEST PASSES

but it misses 16-byte struct passed on registers changes, so it is not ready.

@sandreenko
Copy link
Contributor

With #45467 being merged we have small stack arguments passing support on Apple Arm64, it fixes the following tests from pri0:

Interop/PInvoke/Primitives/Int/PInvokeIntTest/PInvokeIntTest.sh
Loader/binding/tracing/BinderTracingTest.Basic/BinderTracingTest.Basic.sh
JIT/Methodical/explicit/rotate/_opt_relrotarg_objref/_opt_relrotarg_objref.sh
JIT/Methodical/explicit/rotate/_il_relrotarg_double/_il_relrotarg_double.sh
JIT/superpmi/superpmicollect/superpmicollect.sh
profiler/unittest/releaseondetach/releaseondetach.sh
profiler/unittest/metadatagetdispenser/metadatagetdispenser.sh
profiler/unittest/getappdomainstaticaddress/getappdomainstaticaddress.sh
profiler/eventpipe/eventpipe/eventpipe.sh
profiler/eventpipe/eventpipe_readevents/eventpipe_readevents.sh
profiler/gc/gc/gc.sh
profiler/gc/gcbasic/gcbasic.sh
profiler/elt/slowpatheltenter/slowpatheltenter.sh
profiler/elt/slowpatheltleave/slowpatheltleave.sh
ilasm/PortablePdb/IlasmPortablePdbTests/IlasmPortablePdbTests.sh
ilasm/System/Runtime/CompilerServices/MethodImplOptionsTests/MethodImplOptionsTests.sh

I will publish results for pri1 later, I have not seen any regressions, but I suppose there could be some missed cases.

If we ever need to port this support back here are the PRs:
#43024 (lowering->codegen support)
#43130 (importation->morph support)
#42503 (morph->lowering support)
#45461 (a few additions to the previous changes)

#45467 (Enable Arm64 Mac OS ABI.)

@sandreenko
Copy link
Contributor

I extracted HFA(float) support #45780 as a separate issue for a clearer discussion.

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Dec 9, 2020

Closing in favor of #45780

@sdmaclea sdmaclea closed this as completed Dec 9, 2020
@sdmaclea
Copy link
Contributor Author

sdmaclea commented Dec 9, 2020

Looking more closely, the 3 tasks above aren't fully covered by #45780

@sandreenko
Copy link
Contributor

Closing this meta, the last item is covered by #46456

@ghost ghost locked as resolved and limited conversation to collaborators Jan 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Bottom Up Work Not part of a theme, epic, or user story os-macos-bigsur (macOS11) User Story A single user-facing feature. Can be grouped under an epic.
Projects
Archived in project
Development

No branches or pull requests

5 participants