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.

Sign up for GitHub

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

Jump to bottom

APX: Assertion failed 'size == info.compCompHnd->getClassSize(typeHnd)' #113869

Closed
BruceForstall opened this issue Mar 25, 2025 · 8 comments · Fixed by #113901
Closed

APX: Assertion failed 'size == info.compCompHnd->getClassSize(typeHnd)' #113869

BruceForstall opened this issue Mar 25, 2025 · 8 comments · Fixed by #113901
Assignees
@BruceForstall
Labels
apx Related to the Intel Advanced Performance Extensions (APX) area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI in-pr There is an active PR which will close this issue when it is merged
Milestone
10.0.0

Comments

@BruceForstall
Copy link
Member

In runtime-coreclr superpmi-replay-apx pipeline:

https://dev.azure.com/dnceng-public/public/_build/results?buildId=992131&view=results

Assertion failed size == info.compCompHnd->getClassSize(typeHnd)

Subset of examples:

[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\coreclr_tests.run.windows.x64.checked.mch # 515964 : IL size 1
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 211289 : IL size 1
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 211247 : IL size 1
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 211311 : IL size 1
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 106012 : IL size 1
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 14229 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 105889 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 281081 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 199758 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 247850 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 281082 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 106395 : IL size 2
[02:21:19]   C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\libraries.pmi.windows.x64.checked.mch # 7088 : IL size 2

Example superpmi.exe invocation:

C:\h\w\AAE30945\p\superpmi.exe -v ewi -p -jitoption force AltJit=* -jitoption force AltJitNgen=* -jitoption RunAltJitCode=0 -jitoption EnableAPX=1 -f C:\h\w\AAE30945\t\tmpn61746wz\benchmarks.run.windows.x64.checked.mch_fail.mcl -details C:\h\w\AAE30945\t\tmpn61746wz\benchmarks.run.windows.x64.checked.mch_details.csv C:\h\w\AAE30945\p\clrjit_win_x64_x64.dll C:\h\w\AAE30945\p\artifacts\spmi\mch\7139df75-63b2-4610-9a53-f9a86f474db8.windows.x64\benchmarks.run.windows.x64.checked.mch

@anthonycanino @Ruihan-Yin @DeepakRajendrakumaran @tannergooding
@BruceForstall BruceForstall added apx Related to the Intel Advanced Performance Extensions (APX) area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Mar 25, 2025
@BruceForstall BruceForstall added this to the 10.0.0 milestone Mar 25, 2025
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@kunalspathak
Copy link
Member

I think this is a common assert that I ran into while working on agnostic VL support. I am facing this for Vector<T> but might occur for other situations too. When I run arm64 altjit on my x64 box, I always have to set DOTNET_MaxVectorTBitWidth=128. That works because on my x64 box, the VL is 32B and with that environment variable I am downleveling it to 16B, however, if I want to mimic when VL=64B on arm64, I hit this assert because from VM POV (that is on x64), it is always 32B. I need to dig where in VM we can override this.

@BruceForstall
Copy link
Member Author

Is altjit overriding the DOTNET_MaxVectorTBitWidth to some maximum that differs from the collection? Could it just use whatever is in the collection (at least in the case of same-architecture altjit)?

@DeepakRajendrakumaran
Copy link
Contributor

Yes. I ran into the same issue with altjit. Had to set

DOTNET_MaxVectorTBitWidth=128

@kunalspathak
Copy link
Member

Is altjit overriding the DOTNET_MaxVectorTBitWidth to some maximum that differs from the collection? Could it just use whatever is in the collection (at least in the case of same-architecture altjit)?

Just to clarify, my complain is when doing a normal corerun while setting altjit for specific method and not specifically related to replaying superpmi collection (although that might be problematic too).

@kunalspathak
Copy link
Member

Seems this piece of code should be updated to handle AltJit as well as for handling Agnostic VL:

runtime/src/coreclr/vm/methodtablebuilder.cpp

Lines 1126 to 1171 in 7efe7f7

BOOL MethodTableBuilder::CheckIfSIMDAndUpdateSize()
{
STANDARD_VM_CONTRACT;
#if defined(TARGET_X86) || defined(TARGET_AMD64)
if (!bmtProp->fIsIntrinsicType)
return false;
if (bmtFP->NumInstanceFieldBytes != 16)
return false;
LPCUTF8 className;
LPCUTF8 nameSpace;
if (FAILED(GetMDImport()->GetNameOfTypeDef(bmtInternal->pType->GetTypeDefToken(), &className, &nameSpace)))
return false;
if (strcmp(className, "Vector`1") != 0 || strcmp(nameSpace, "System.Numerics") != 0)
return false;
CORJIT_FLAGS CPUCompileFlags = ExecutionManager::GetEEJitManager()->GetCPUCompileFlags();
uint32_t numInstanceFieldBytes = 16;
if (CPUCompileFlags.IsSet(InstructionSet_VectorT512))
{
numInstanceFieldBytes = 64;
}
else if (CPUCompileFlags.IsSet(InstructionSet_VectorT256))
{
numInstanceFieldBytes = 32;
}
if (numInstanceFieldBytes != 16)
{
bmtFP->NumInstanceFieldBytes = numInstanceFieldBytes;
if (HasLayout())
{
GetLayoutInfo()->m_cbManagedSize = numInstanceFieldBytes;
}
return true;
}
#endif // TARGET_X86 || TARGET_AMD64
return false;

@jkotas
Copy link
Member

jkotas commented Mar 25, 2025

Seems this piece of code should be updated to handle AltJit

If you update this to produce value that works for cross-architecture AltJit, I expect that it is going to break regular native Jit.

So far, the AltJit quirks have been done in the JIT itself or on the JIT/EE interface at most.

@BruceForstall BruceForstall self-assigned this Mar 25, 2025
BruceForstall added a commit to BruceForstall/runtime that referenced this issue Mar 25, 2025
@BruceForstall
Fix AltJit and VectorT ISA specification
1ed1f17 
When running as an AltJit when the generated code will not be used
(either because of a mismatched VM, or because RunAltJitCode=0),
the JIT overwrites the passed in ISAs. When it does so, it zeros
out the VectorT* ISA flags.

Instead of this, carry over the existing ISA flags. This should work
for same-architecture SPMI replays.

I'm not sure if additional logic for specifically setting the VectorT*
ISA is required for other scenarios, or if incorporating similar logic
to `EEJitManager::SetCpuInfo()` into `Compiler::compCompile()` is
required.

Adds new JIT debugging helpers `dIsa()` and `dIsaFlags()` to dump out
string representations of `CORINFO_InstructionSet` and
`CORINFO_InstructionSetFlags`.

Fixes dotnet#113869
@BruceForstall BruceForstall mentioned this issue Mar 25, 2025
Merged
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Mar 25, 2025
@BruceForstall
Copy link
Member Author

I put up a PR to hopefully fix this (or at least work around it in this scenario) with #113901

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BruceForstall BruceForstall

Labels
apx Related to the Intel Advanced Performance Extensions (APX) area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Milestone
10.0.0
Development

Successfully merging a pull request may close this issue.

Fix AltJit and VectorT ISA specification BruceForstall/runtime
4 participants
@jkotas @BruceForstall @kunalspathak @DeepakRajendrakumaran