runtime: crossgen2: getHFAType fails on arm64 and passes on x64 targetting arm64

https://github.com/dotnet/runtime/blob/78efb9250245617ff8532371ae4b8375a741f5ea/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs#L2540 fails on arm64 and passes on x64 targetting arm64.

To repro get this change: https://github.com/sandreenko/runtime/commit/880a563af8171609257c80045538d98401aa8877

readytorun\crossgen2\crossgen2smoke

and run crossgen2 test on x64 -> passes, on x64 targeting arm64 -> passes, on arm64 -> fails with:

Z:\PayloadGroup0>dotnet "Z:\CoreRoot\crossgen2\crossgen2.dll" @"Z:\PayloadGroup0\readytorun\crossgen2\crossgen2smoke\\composite-r2r.dll.rsp"   --composite"
To repro, add following arguments to the command line:
--singlemethodtypename "HelperGenericILCode`1[[System.__Canon]],helperildll" --singlemethodname LdTokenArrayMethods --singlemethodindex 1
Unhandled exception. ILCompiler.CodeGenerationFailedException: Code generation failed for method '[helperildll]HelperGenericILCode`1<System.__Canon>.LdTokenArrayMethods(RuntimeMethodHandle&,RuntimeMethodHandle&,RuntimeMethodHandle&,RuntimeMethodHandle&,RuntimeMethodHandle&)'
   at Internal.JitInterface.CorInfoImpl.CompileMethodInternal(IMethodNode methodCodeNodeNeedingCode, MethodIL methodIL) in /_/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs:line 216
   at Internal.JitInterface.CorInfoImpl.CompileMethod(MethodWithGCInfo methodCodeNodeNeedingCode) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs:line 391
   at ILCompiler.ReadyToRunCodegenCompilation.<ComputeDependencyNodeDependencies>b__26_0(DependencyNodeCore`1 dependency) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 499
   at ILCompiler.ReadyToRunCodegenCompilation.ComputeDependencyNodeDependencies(List`1 obj) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 525
   at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 302
   at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 302
   at ILCompiler.Program.Run(String[] args) in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 590
   at ILCompiler.Program.Main(String[] args) in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 739

for the minimal repro use the following rsp file(with your passes):

Y:\PayloadGroup0\readytorun\crossgen2\crossgen2smoke\IL-CG2\*.dll
-o:Y:\PayloadGroup0\readytorun\crossgen2\crossgen2smoke\\composite-r2r.dll
--targetarch:arm64
--verify-type-and-field-layout
-O
-r:Y:\CoreRoot\System.*.dll
-r:Y:\CoreRoot\Microsoft.*.dll
-r:Y:\CoreRoot\mscorlib.dll
-r:Y:\CoreRoot\netstandard.dll
--parallelism 
1
--singlemethodtypename 
HelperGenericILCode`1[[System.__Canon]],helperildll
--singlemethodname
LdTokenArrayMethods
--singlemethodindex
1

Remove a workaround for https://github.com/dotnet/runtime/pull/46034 when it is fixed.

About this issue

Original URL
State: open
Created 4 years ago
Comments: 19 (19 by maintainers)

Most upvoted comments

Hmm, I thought this information was readily available to JIT without making another interface call, if not, you’re certainly right, we don’t want to add another interface call for this purpose.

‘isValueType’ is saved in LclVars and GenTrees that call ‘getHFAType’ method so it is cheap to fix. Maybe it would ever be a speed-up not to call it when JIT-EE already knows the result.

cc @jakobbotsch

sandreenko on Jun 8, 2023

@ivdiazsa - I think that part of the trick is fixing Sergey’s JIT change to only call the JIT interface method for valuetypes; the assert in jitinterface.cpp will just make sure that we crash once someone violates it in JIT again in the future.

trylek on Jun 7, 2023

The comment on getHFAType JIT/EE interface method says: Returns type of HFA for valuetype

The crossgen implementation of this method assumes that this method can be only called on valuetypes. https://github.com/sandreenko/runtime/commit/880a563af8171609257c80045538d98401aa8877 violates the assumption and that’s why you are seeing the crash.

We can either:

Change the method contract to make it callable on any type: Adjust the comment, adjust the crossgen implementation (return false), apply the proposed change in the JIT. or
Keep the method contract as is (ie treat the current behavior as “by design”). It would be nice to add assert to vm/jitinterface.cpp to ensure that this method is only called for valuetypes, so that we are not seeing these problems in crossgen and AOT only.

jkotas on Jun 6, 2023

From a quick pass over the callers I think it will suffice to add a varTypeIsStruct(argSigType) guard here: https://github.com/dotnet/runtime/blob/3fded0bb77c852b10d712bd629ced740499a3aa6/src/coreclr/jit/morph.cpp#L2235

All other callers I saw seem to be from paths that already know we are dealing with a value type.

jakobbotsch on Jun 8, 2023

This issue is apparently still happening. I’m currently investigating.

ivdiazsa on Jun 5, 2023

We discussed that offline. The current behavior may be considered “by design”. There was another question why compiling for ARM64 on x64 and ARM64 machines demonstrated different behavior (no assert vs. assert). I can check whether it is still the case. I think we can safely move it to future release.

AntonLapounov on Jul 9, 2021