[wasm][R2R] Fix Vector128 argument alignment in crossgen ArgIterator

docs/design/datacontracts/RuntimeTypeSystem.md

@@ -84,6 +84,12 @@ partial interface IRuntimeTypeSystem : IContract
     // define FEATURE_HFA). Mirrors MethodTable::GetHFAType in
     // src/coreclr/vm/class.cpp.
     public virtual bool TryGetHFAElementSize(ITypeHandle typeHandle, out int elementSize);
+    // Returns the intrinsic SIMD vector element size derived from type metadata: 16 for
+    // Vector128<T>, 8 for Vector64<T>, and 8 or 16 for System.Numerics.Vector<T> (based on its
+    // instance size); 0 for non-vector types and for Vector256<T>/Vector512<T> (never HVA
+    // elements). Unlike TryGetHFAElementSize this is not gated on FEATURE_HFA, so it is valid on
+    // wasm, where ArgIterator uses it to 16-byte align v128 args.
+    public virtual int GetVectorElementSize(ITypeHandle typeHandle);
     // True if the type requires 8-byte alignment on platforms that don't 8-byte align by default (FEATURE_64BIT_ALIGNMENT)
     public virtual bool RequiresAlign8(ITypeHandle typeHandle);
     // Returns the cached SystemV AMD64 eightbyte register-passing classification for a value type
@@ -806,7 +812,7 @@ static class RuntimeTypeSystem_1_Helpers
     //       if targetArch is ARM:                         return (true, th.Flags.RequiresAlign8 ? 8 : 4)
     //       mt = th
     //       loop (bounded depth):
-    //           if (elem = GetVectorHFAElementSize(mt)):  return (true, elem)
+    //           if (elem = GetVectorElementSize(mt)):      return (true, elem)
     //           field = first non-static field of mt
     //           if field is null:                         return false
     //           switch field.ElementType:
@@ -815,18 +821,22 @@ static class RuntimeTypeSystem_1_Helpers
     //               ValueType: mt = GetFieldDescApproxTypeHandle(field); continue
     //               default:   return false
     //
-    //   GetVectorHFAElementSize(mt):                      // detects HVA shapes
+    //   GetVectorElementSize(mt):                         // detects HVA / SIMD vector shapes
     //       if !mt.Flags.IsIntrinsicType:                 return 0
     //       (ns, name) = typedef name+namespace via EcmaMetadata
     //       elem = match on (ns, name):
     //           "System.Numerics", "Vector`1":            NumInstanceFieldBytes (8 or 16, else 0)
     //           "System.Runtime.Intrinsics", "Vector128`1": 16
     //           "System.Runtime.Intrinsics", "Vector64`1":  8
-    //           _:                                          return 0
+    //           _:                                          return 0    // incl. Vector256/512
     //       if !CorIsNumericalType(GetInstantiation(mt)[0]): return 0
     //       return elem
     public bool TryGetHFAElementSize(ITypeHandle typeHandle, out int elementSize) { ... }
 
+    // Metadata-derived intrinsic vector element size, without FEATURE_HFA gating so it is usable
+    // on wasm (see GetVectorElementSize pseudocode above).
+    public int GetVectorElementSize(ITypeHandle typeHandle) { ... }
+
     public bool RequiresAlign8(ITypeHandle typeHandle) => !typeHandle.IsMethodTable() ? false : _methodTables[typeHandle.Address].Flags.RequiresAlign8;
 
     public bool IsCanonicalMethodTable(ITypeHandle typeHandle)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Wasm.cs

@@ -5,6 +5,8 @@
 
 using Internal.TypeSystem;
 
+using Debug = System.Diagnostics.Debug;
+
 namespace ILCompiler
 {
     public partial class CompilerTypeSystemContext
@@ -16,15 +18,23 @@ public partial class CompilerTypeSystemContext
 
         /// <summary>
         /// Gets the first SIMD v128 type encountered during lowering, or null if none has been seen.
-        /// Used by RaiseSignature to produce a roundtrippable type for the 'V' encoding.
+        /// Used by RaiseSignature to produce a roundtrippable type for the 'V' encoding. Any v128
+        /// type is usable there because all of them share the same wasm ABI (see CacheV128Type).
         /// </summary>
         public TypeDesc CachedV128Type => _cachedV128Type;
 
         /// <summary>
-        /// Caches a SIMD v128 type discovered during lowering. Only the first one is retained.
+        /// Caches a SIMD v128 type discovered during lowering. Only the first one is retained:
+        /// every type that lowers to a wasm <c>v128</c> is 16 bytes with 16-byte alignment, so they
+        /// are interchangeable for the signature round-trip that RaiseSignature performs. The assert
+        /// guards that invariant, since a v128 type with a smaller alignment would silently give
+        /// raised signatures a different argument layout depending on which type was lowered first.
         /// </summary>
         public void CacheV128Type(TypeDesc type)
         {
+            Debug.Assert(((DefType)type).InstanceFieldAlignment.AsInt == 16,
+                $"v128 type {type} must be 16-byte aligned to be interchangeable in raised signatures");
+
             _cachedV128Type ??= type;
         }
 

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs

@@ -311,7 +311,13 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType type,
                 {
                     ByteCountUnaligned = layoutFromSimilarIntrinsicVector.ByteCountUnaligned,
                     ByteCountAlignment = layoutFromMetadata.ByteCountAlignment,
-                    FieldAlignment = layoutFromMetadata.FieldAlignment,
+                    // On wasm Vector<T> is passed as a v128, exactly like the similar intrinsic
+                    // vector, so it has to share that type's 16-byte alignment. Elsewhere Vector<T>
+                    // does not follow the intrinsic vector calling convention yet, and keeps the
+                    // alignment its metadata layout produces (see MATCHING_HARDWARE_VECTOR above).
+                    FieldAlignment = type.Context.Target.Architecture == TargetArchitecture.Wasm32
+                        ? layoutFromSimilarIntrinsicVector.FieldAlignment
+                        : layoutFromMetadata.FieldAlignment,
                     FieldSize = layoutFromSimilarIntrinsicVector.FieldSize,
                     Offsets = layoutFromMetadata.Offsets,
                     LayoutAbiStable = true,

src/coreclr/vm/methodtablebuilder.cpp

@@ -10700,6 +10700,18 @@ void MethodTableBuilder::CheckForSystemTypes()
 
                 return;
             }
+
+#ifdef TARGET_WASM
+            // System.Numerics.Vector<T> is a v128 value on wasm, so it needs the same 16-byte
+            // alignment as System.Runtime.Intrinsics.Vector128<T> above. Its metadata layout is
+            // already 16 bytes (two UInt64 fields), but those only give it 8-byte alignment,
+            // which disagrees with crossgen2 and the interpreter.
+            if ((strcmp(nameSpace, g_NumericsNS) == 0) && (strcmp(name, "Vector`1") == 0))
+            {
+                pClass->GetLayoutInfo()->SetAlignmentRequirement(16); // sizeof(v128)
+                return;
+            }
+#endif // TARGET_WASM
         }
 
         if (g_pNullableClass != NULL)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeTypeSystem.cs

@@ -178,6 +178,13 @@ public interface IRuntimeTypeSystem : IContract
     // define FEATURE_HFA). Mirrors MethodTable::GetHFAType in
     // src/coreclr/vm/class.cpp.
     bool TryGetHFAElementSize(ITypeHandle typeHandle, out int elementSize) => throw new NotImplementedException();
+    // Returns the intrinsic SIMD vector element size derived from type metadata: 16 for
+    // Vector128<T>, 8 for Vector64<T>, and 8 or 16 for System.Numerics.Vector<T> (based on its
+    // instance size). Returns 0 for non-vector types, and also for Vector256<T>/Vector512<T>
+    // (never HVA elements; no caller needs their size). Unlike TryGetHFAElementSize this is not
+    // gated on FEATURE_HFA, so it is valid on wasm, where ArgIterator uses it to give v128
+    // (16-byte) arguments the stack alignment the runtime and interpreter require.
+    int GetVectorElementSize(ITypeHandle typeHandle) => throw new NotImplementedException();
     // True if the type requires 8-byte alignment on platforms that don't 8-byte align by default (FEATURE_64BIT_ALIGNMENT)
     bool RequiresAlign8(ITypeHandle typeHandle) => throw new NotImplementedException();
     // Returns the cached SystemV AMD64 eightbyte register-passing classification for a value type

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CallingConvention/CdacTypeHandle.cs

@@ -229,9 +229,17 @@ public bool IsTrivialPointerSizedStruct()
         }
     }
 
-    // Only used by ArgIterator on WASM32 for stack alignment of value types.
+    // Only used by ArgIterator on WASM32 for stack alignment of value types. Vector128<T> /
+    // 128-bit Vector<T> are 16-byte aligned by the runtime and interpreter ArgIterator
+    // (getClassAlignmentRequirement), so report 16 for the wasm v128 SIMD types to reconstruct
+    // the same argument layout. Non-v128 value types are not yet reconstructed by the cDAC reader.
     public int GetFieldAlignment()
     {
+        if (_typeHandle is not null && Rts.GetVectorElementSize(_typeHandle) == 16)
+        {
+            return 16;
+        }
+
         throw new NotImplementedException("Field alignment is not yet implemented.");
     }
 

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs

@@ -693,7 +693,7 @@ public bool TryGetHFAElementSize(ITypeHandle typeHandle, out int elementSize)
         ITypeHandle current = typeHandle;
         for (int depth = 0; depth < 16; depth++)
         {
-            int vectorElem = GetVectorHFAElementSize(current);
+            int vectorElem = GetVectorElementSize(current);
             if (vectorElem != 0)
             {
                 elementSize = vectorElem;
@@ -735,9 +735,13 @@ public bool TryGetHFAElementSize(ITypeHandle typeHandle, out int elementSize)
         return false;
     }
 
-    // Mirrors MethodTable::GetVectorHFA in src/coreclr/vm/class.cpp. Any
-    // metadata decode failure returns 0 (treated as "not an HVA").
-    private int GetVectorHFAElementSize(ITypeHandle typeHandle)
+    // Metadata-derived intrinsic vector element size, independent of FEATURE_HFA gating so it is
+    // usable on wasm (where ArgIterator uses it to 16-byte align v128 arguments) as well as by the
+    // HFA/HVA classification above. Returns 8 for Vector64<T> and 8-byte Vector<T>, 16 for
+    // Vector128<T> and 16-byte Vector<T>, and 0 for anything else. Vector256<T>/Vector512<T> return
+    // 0: they are never HVA elements and no caller needs their size. Mirrors
+    // MethodTable::GetVectorHFA in src/coreclr/vm/class.cpp. Any metadata decode failure returns 0.
+    public int GetVectorElementSize(ITypeHandle typeHandle)
     {
         if (!typeHandle.IsMethodTable() || !_methodTables[typeHandle.Address].Flags.IsIntrinsicType)
             return 0;

src/native/managed/cdac/tests/StressTests/Debuggees/CallSignatures/Program.cs

@@ -532,7 +532,7 @@ private struct NestedDouble4 { public Double2 First; public Double2 Second; }
 
     // --- HVA shapes (intrinsic Vector types). On non-FEATURE_HFA targets
     //     these go through the regular struct path; on ARM64 they hit the
-    //     GetVectorHFAElementSize TypeDef-name match (Vector64/128 in
+    //     GetVectorElementSize TypeDef-name match (Vector64/128 in
     //     System.Runtime.Intrinsics, Vector<T> in System.Numerics). ---
     [MethodImpl(MethodImplOptions.NoInlining)] private static void Vec64FloatArg(Vector64<float> v) { AllocBurst(); GC.KeepAlive((object)v.GetElement(0)); }
     [MethodImpl(MethodImplOptions.NoInlining)] private static void Vec128FloatArg(Vector128<float> v) { AllocBurst(); GC.KeepAlive((object)v.GetElement(0)); }