Repository navigation
crossgen2: getHFAType fails on arm64 and passes on x64 targetting arm64 #46126
Description
fails on arm64 and passes on x64 targetting arm64.
To repro get this change:
sandreenko@880a563
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 #46034 when it is fixed.
Activity
sandreenko commentedon Dec 16, 2020
PTAL @mangod9 @jkotas
mangod9 commentedon Dec 16, 2020
cc @dotnet/crossgen-contrib. @sandreenko, is this blocking your PR from merging?
sandreenko commentedon Dec 16, 2020
@mangod9 no, it does not
mangod9 commentedon Dec 16, 2020
Thanks, assigning to @AntonLapounov who will look at it in Jan.
mangod9 commentedon Jul 9, 2021
assume this is still something that needs investigation?
AntonLapounov commentedon Jul 9, 2021
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.
6 remaining items
sandreenko commentedon Jun 7, 2023
The assert and change in Jit to call it only on value types sound reasonable.
Probably many methods in jit call it right now without a value type check, like
runtime/src/coreclr/jit/codegencommon.cpp
Line 6482 in 652433b
GetHfaType(NO_CLASS_HANDLE)returnsTYP_UNDEFbut the fix should not be complex.trylek commentedon Jun 7, 2023
I guess it should suffice to fix the JIT wrapper
GetHfaTypeto avoid actually calling the JIT interface unless the type is a valuetype, there are various sorts of upfront checks there already.jkotas commentedon Jun 7, 2023
If you need to do another call on JIT/EE inteface to check whether it is a value type, it is more efficient to allow passing any types into the JIT/EE interface getHFAType API.
trylek commentedon Jun 7, 2023
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.
sandreenko commentedon Jun 8, 2023
'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
jakobbotsch commentedon Jun 8, 2023
Some callers of
GetHfaTypealready guard onvarTypeIsStruct(type)for the corresponding JIT type, so it would make sense to me to do this more thoroughly. Alternatively we can changeGetHfaTypeto takeClassLayout*that indeed already has cachedisValueTypeavailable. That might also save agetClassSizeJIT-EE call in a few places.jakobbotsch commentedon Jun 8, 2023
From a quick pass over the callers I think it will suffice to add a
varTypeIsStruct(argSigType)guard here:runtime/src/coreclr/jit/morph.cpp
Line 2235 in 3fded0b
All other callers I saw seem to be from paths that already know we are dealing with a value type.
AndyAyersMS commentedon Aug 11, 2026
This no longer reproduces with the current Crossgen2 and JIT. The
crossgen2smokeinputs now compile successfully as a composite ARM64 ReadyToRun image from an x64 host.The unguarded
GetHfaType(argSigClass)call identified in the final discussion was removed by #112665. Current ARM and ARM64 ABI classifiers only query HFA information for struct types.Closing as fixed by the subsequent ABI classification cleanup.