Skip to content

[wasm] Use ordinary struct alignment for Int128/UInt128 on wasm #131421

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

Closed
lewing wants to merge 2 commits into from
+16 −2
Closed

[wasm] Use ordinary struct alignment for Int128/UInt128 on wasm #131421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6189b46
[wasm] Use ordinary struct alignment for Int128/UInt128 on wasm
lewing Jul 27, 2026
5fe365d
Add Wasm32 cases to the Int128/UInt128 architecture layout tests
lewing Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/Compiler/Int128FieldLayoutAlgorithm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public override ComputedInstanceFieldLayout ComputeInstanceLayout(DefType defTyp
ComputedInstanceFieldLayout layoutFromMetadata = _fallbackAlgorithm.ComputeInstanceLayout(defType, layoutKind);

// 32bit platforms use standard metadata layout engine
if (defType.Context.Target.Architecture == TargetArchitecture.ARM)
if (defType.Context.Target.Architecture is TargetArchitecture.ARM or TargetArchitecture.Wasm32)
{
Comment on lines 33 to 35

@lewing lewing Jul 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct that the comment is imprecise — X86 is 32-bit and does take the 16-byte override. That imprecision predates this change though, and the condition it describes is right there on the next line, so I'd rather not expand it here: rewording a pre-existing comment is unrelated churn, and per repo convention comments are kept minimal because inaccurate ones mislead and need maintenance.

Happy to fix it if a maintainer would prefer.

Note

This comment was generated by GitHub Copilot.

layoutFromMetadata.LayoutAbiStable = true;
layoutFromMetadata.IsInt128OrHasInt128Fields = true;
Comment on lines +34 to 37

@lewing lewing Jul 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Wasm32 cases to TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence in 5fe365d. Both fail without the layout change (alignment 16 rather than 8), so they actually guard the new branch rather than just documenting it.

Note

This comment was generated by GitHub Copilot.

Expand Down
13 changes: 13 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/ArchitectureSpecificFieldLayoutTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ public class ArchitectureSpecificFieldLayoutTests
private ModuleDesc _testModuleARM;
private TestTypeSystemContext _contextARM64;
private ModuleDesc _testModuleARM64;
private TestTypeSystemContext _contextWasm32;
private ModuleDesc _testModuleWasm32;

public ArchitectureSpecificFieldLayoutTests()
{
Expand Down Expand Up @@ -60,6 +62,12 @@ public ArchitectureSpecificFieldLayoutTests()
_contextARM64.SetSystemModule(systemModuleARM64);

_testModuleARM64 = systemModuleARM64;

_contextWasm32 = new TestTypeSystemContext(TargetArchitecture.Wasm32);
var systemModuleWasm32 = _contextWasm32.CreateModuleForSimpleName("CoreTestAssembly");
_contextWasm32.SetSystemModule(systemModuleWasm32);

_testModuleWasm32 = systemModuleWasm32;
}

[Fact]
Expand Down Expand Up @@ -501,11 +509,13 @@ public void TestAlignmentBehavior_AutoAlignmentRules(string wrapperType, int[] a
[InlineData("StructStructByte_Int128StructAuto", "X86", 16, 32)]
[InlineData("StructStructByte_Int128StructAuto", "X64Linux", 16, 32)]
[InlineData("StructStructByte_Int128StructAuto", "X64Windows", 16, 32)]
[InlineData("StructStructByte_Int128StructAuto", "Wasm32", 8, 24)]
[InlineData("StructStructByte_UInt128StructAuto", "ARM64", 16, 32)]
[InlineData("StructStructByte_UInt128StructAuto", "ARM", 8, 24)]
[InlineData("StructStructByte_UInt128StructAuto", "X86", 16, 32)]
[InlineData("StructStructByte_UInt128StructAuto", "X64Linux", 16, 32)]
[InlineData("StructStructByte_UInt128StructAuto", "X64Windows", 16, 32)]
[InlineData("StructStructByte_UInt128StructAuto", "Wasm32", 8, 24)]
// Variation of TestAlignmentBehavior_AutoAlignmentRules above that is able to deal with os specific behavior
public void TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence(string wrapperType, string osArch, int alignment, int size)
{
Expand All @@ -530,6 +540,9 @@ public void TestAlignmentBehavior_AutoAlignmentRulesWithOSDependence(string wrap
case "X86":
testModule = _testModuleX86;
break;
case "Wasm32":
testModule = _testModuleWasm32;
break;
default:
throw new Exception();
}
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/methodtablebuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10817,7 +10817,8 @@ void MethodTableBuilder::CheckForSystemTypes()
// even on X86
pLayout->SetAlignmentRequirement(16); // sizeof(__int128)
#elif defined(TARGET_WASM)
pLayout->SetAlignmentRequirement(16); // sizeof(v128)
// Wasm has no 128-bit scalar type to match, so use the natural field alignment.
pLayout->SetAlignmentRequirement(8);
#else
#error Unknown architecture
#endif // TARGET_64BIT
Comment on lines +10820 to 10824
Expand Down
Loading