Handle Decimal32/64/128 ABI packing in the VM and AOT type system#131345
Handle Decimal32/64/128 ABI packing in the VM and AOT type system#131345tannergooding wants to merge 3 commits into
Conversation
The IEEE 754 decimal types had no special ABI handling, so Decimal128 defaulted to 8 byte alignment rather than the 16 byte alignment of the _Decimal128 ABI primitive, and by-value interop passing hit the same unimplemented JIT path as Int128/UInt128. Mirror the Int128 handling with a parallel flag, and mark the types [Intrinsic] so the layout algorithm and native alignment propagation can key off it the same way. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/interop-contrib |
|
CC. @lewing, @jkotas, @jkoritzinsky. We added the Decimal32/64/128 types in .NET 11 and they are ABI primitives with special packing and ABI conventions, so as has been standard we're ensuring the managed packing is correct and then blocking them from crossing the P/Invoke boundary until the right classification can be enabled for all platforms (namely classifying as SSE/SSEUP on SysV). |
There was a problem hiding this comment.
Pull request overview
Updates CoreCLR + NativeAOT type system + interop marshalling to treat System.Numerics.Decimal32/64/128 as ABI-special types: enforce 16-byte packing for Decimal128 where appropriate and block by-value P/Invoke marshalling (throwing MarshalDirectiveException) for all three, mirroring existing Int128/UInt128 handling.
Changes:
- Mark
Decimal32/64/128as[Intrinsic]and ensure layout metadata matches runtime expectations. - Add a new “decimal floating-point (or has decimal fields)” flag across VM and shared type system, using it to (a) adjust
Decimal128alignment and (b) reject by-value marshalling. - Add new interop tests and native test asset for
Decimal128alignment + by-value marshalling restriction coverage.
Show a summary per file
| File | Description |
|---|---|
| src/tests/Interop/PInvoke/Decimal128/Decimal128Test.cs | Adds managed interop tests for Decimal128 layout + by-value restriction, plus Decimal32/64 restriction checks |
| src/tests/Interop/PInvoke/Decimal128/Decimal128Native.cpp | Native side helpers to validate field layout/alignment and host dummy exports for restriction tests |
| src/tests/Interop/PInvoke/Decimal128/CMakeLists.txt | Builds the new native interop test library |
| src/tests/Interop/Interop.csproj | Includes new Decimal128 test sources in the merged wrapper build |
| src/tests/Interop/CMakeLists.txt | Adds the Decimal128 native test subdirectory to the interop native build |
| src/libraries/System.Private.CoreLib/src/System/Numerics/Decimal32.cs | Marks Decimal32 as intrinsic (and confirms sequential layout attribute) |
| src/libraries/System.Private.CoreLib/src/System/Numerics/Decimal64.cs | Marks Decimal64 as intrinsic and adds explicit sequential layout |
| src/libraries/System.Private.CoreLib/src/System/Numerics/Decimal128.cs | Marks Decimal128 as intrinsic and adds explicit sequential layout |
| src/coreclr/vm/mlinfo.cpp | Blocks by-value marshalling for decimal floating-point types/containers with a dedicated resource ID |
| src/coreclr/vm/methodtablebuilder.cpp | Tracks decimal-floating-point containment and sets Decimal128 alignment requirement by target |
| src/coreclr/vm/methodtable.inl | Adds MethodTable::IsDecimalFloatingPointOrHasDecimalFloatingPointFields() helper |
| src/coreclr/vm/methodtable.h | Declares the new MethodTable query helper |
| src/coreclr/vm/jitinterface.cpp | Ensures intrinsic decimal types in System.Numerics still report fields to the JIT layout API |
| src/coreclr/vm/dllimport.cpp | Forces marshaling-required for decimal floating-point types/containers (so restriction triggers) |
| src/coreclr/vm/corelib.h | Adds CoreLib binder entry for Decimal128 for native layout handling |
| src/coreclr/vm/classnames.h | Adds Decimal32/64/128 name constants for VM type recognition |
| src/coreclr/vm/classlayoutinfo.cpp | Propagates decimal containment via nested-field flags and aligns Decimal128 native layout with managed |
| src/coreclr/vm/class.h | Adds EEClassLayoutInfo and EEClass flag plumbing for decimal containment |
| src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.cs | Rejects by-value marshalling in ILCompiler type system for decimal-floating-point types/containers |
| src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs | Propagates the decimal containment bit through computed layouts |
| src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutAlgorithm.cs | Extends computed layout struct with decimal containment flag |
| src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs | Adds a persistent layout flag + public query property for decimal containment |
| src/coreclr/tools/Common/Compiler/DecimalFieldLayoutAlgorithm.cs | New layout algorithm for Decimal32/64/128; applies 16-byte alignment for Decimal128 where needed |
| src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/TestTypeSystemContext.cs | Wires DecimalFieldLayoutAlgorithm into the test type system context |
| src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ILCompiler.TypeSystem.Tests.csproj | Includes DecimalFieldLayoutAlgorithm in type system tests compilation |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj | Includes DecimalFieldLayoutAlgorithm in R2R build |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs | Selects decimal layout algorithm for decimal floating-point intrinsic types |
| src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj | Includes DecimalFieldLayoutAlgorithm in AOT compiler build |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs | Selects decimal layout algorithm for decimal floating-point intrinsic types |
| src/coreclr/dlls/mscorrc/resource.h | Adds new resource ID for the decimal by-value marshalling restriction |
| src/coreclr/dlls/mscorrc/mscorrc.rc | Adds the user-facing error string for the restriction |
Copilot's findings
- Files reviewed: 31/31 changed files
- Comments generated: 1
The alignas(16) was guarded on MSVC-only _M_* macros, so on Linux/macOS the native Decimal128 kept 8 byte alignment while the managed side used 16, moving the field offset in StructWithDecimal128 and breaking the byref layout check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Mono implements neither the 16-byte packing for ABI primitive types nor the by-value marshaling restriction, so both tests fail there. Covered by the same issue used for the equivalent Int128 test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Had to apply the same "skip on mono" label we have for the other interop tests and updated #69399 to include it in the list |
System.Numerics.Decimal32/64/128were added without any of the special ABI handling thatInt128/UInt128get, so:Decimal128(twoulongs) defaulted to 8 byte alignment rather than the 16 byte alignment of the_Decimal128ABI primitive.Int128/UInt128(ABI for Int128 and UInt128 is incorrect with regards to native ABI for by value parameter passing #74209), silently producing wrong codegen rather than a diagnostic.This mirrors the existing
Int128handling across the VM, the shared crossgen2/ILCompiler type system, and the interop marshallers, using a parallelIsDecimalFloatingPointOrHasDecimalFloatingPointFieldsflag rather than reusing theInt128one -- a decimal isn't anInt128, the diagnostic text differs, and reusing it would be confusing next toSystem.Decimal. ExistingInt128paths are untouched.Two separable pieces:
Decimal128only.Decimal32/Decimal64map ontouint/ulongand keep their natural alignment.MarshalDirectiveExceptionrather than bad codegen, and is tracked by ABI for Decimal32/64/128 is incorrect with regards to native ABI for by value parameter passing #131341 (the decimal analogue of ABI for Int128 and UInt128 is incorrect with regards to native ABI for by value parameter passing #74209). There is also no decimal floating-point calling convention forDecimal32/Decimal64today.The types are now marked
[Intrinsic], matchingInt128/UInt128, soDecimalFieldLayoutAlgorithm.IsDecimalFloatingPointTypecan requiretype.IsIntrinsicexactly likeInt128FieldLayoutAlgorithm.IsIntegerTypedoes. I audited everyIsIntrinsicType()consumer in the VM and everyisIntrinsicType/CORINFO_FLG_INTRINSIC_TYPEsite in the JIT; all are exact-name gated except two, and both mattered:classlayoutinfo.cppGetNativeLayoutInfocopies the managed alignment to the native alignment forInt128/UInt128/Vector*, gated onIsIntrinsicType(). Without[Intrinsic]this was unreachable for the decimals, so a marshaled struct containing aDecimal128would have gotten 8 byte native alignment against its 16 byte managed alignment. AddedCLASS__DECIMAL128.jitinterface.cppGetTypeLayoutHelpersetssimdTypeHndand stops enumerating fields for any intrinsic type inSystem.Numerics-- a namespace-only check that held only because every such type was SIMD until now. The decimals would have been reported withnumFields = 0, andTryPromoteValueClassAsPrimitivewould then bail, silently losing struct promotion for e.g. aDecimal64field that promotes asTYP_LONGtoday. Excluded via the new flag, which combined withIsIntrinsicType()identifies exactly the three types (a user struct merely containing a decimal isn't intrinsic).On the platform split for the 16 byte alignment: ARM32 gets 8, matching
Int128, since its PCS has no such primitive. x64/x86 get 16 (_Decimal128is 16 byte aligned in the x86-64 psABI). Wasm gets 16, but not for thesizeof(v128)reason theInt128branch cites -- clang has no_Decimal128at all, so the Wasm Basic C ABI doesn't define one; 16 matches__int128_t, the only other 16 byte scalar it does define, which stays 16 byte aligned even under Emscripten (Emscripten only reduceslong doubleto 8). The comment says so rather than repeating the inherited rationale.Decimal128's#if BIGENDIAN_upper/_lowerswap was already present and matchesInt128. Mono has noInt128special-casing, so nothing is owed there.[StructLayout(LayoutKind.Sequential)]was added toDecimal64/Decimal128, whichDecimal32andInt128already carried; no ref assembly change, since neitherInt128norDecimal128declares it in the ref and Roslyn defaults structs to sequential either way.New interop tests under
src/tests/Interop/PInvoke/Decimal128:TestDecimal128FieldLayout-- 16 byte alignment round-trip, byref/out marshalling, and the by-valueMarshalDirectiveExceptionboth directly and struct-wrapped.TestDecimal32And64MarshalRestriction-- the marshal restriction for the other two.The native side treats
Decimal128asalignas(16) struct { uint64_t lower, upper; }and adds the raw halves, since MSVC has no_Decimal128either. The managed side builds and reads known bit patterns throughUnsafe.As<UInt128, Decimal128>, asDecimal128has no public bits constructor; that's endian-safe because both types use the same#if BIGENDIANfield swap.Validation:
clrandlibs.pretestbuild clean (0 warnings / 0 errors); the new interop tests pass (exit code 100);System.Runtime.TestsDecimal32/Decimal64/Decimal128-- 7148 total, 0 failed. Nosrc/coreclr/jitfiles changed, so jit-format doesn't apply.CC. @dotnet/jit-contrib @dotnet/interop-contrib
Note
This pull request description was drafted by GitHub Copilot.