Skip to content

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Jul 9, 2025

For untiered code, we don't use the var offset allocator, the offset for vars is computed on the fly as we push and pop them from the execution stack during initial IL import compilation phase. These offsets (and their alignment) follow the same rules as if they would be passed as arguments to a method. When we initially push a var we don't know if this var will be an argument to a call, so it might not have the stack alignment of 16 bytes. In this case, at the moment of a call we add a MINT_MOV_STACK_UNOPT opcode which moves the entire var space so it has the right alignment. The problem is that all simd params (which need 16byte alignment) will now become unaligned. interp_realign_simd_params is called to resolve this problem by doing another mov, this time starting at the location of the first simd argument.

This realignment is meant to be done only once, because once a simd argument is properly aligned, all following arguments are aligned as well, due to the way they are initially pushed on the execution stack. The issue was that we were not breaking the loop once we do the realignment starting at the first simd argument.

Fixes #110644

@Copilot Copilot AI review requested due to automatic review settings July 9, 2025 20:05
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where subsequent SIMD arguments remained unaligned by ensuring realignment only happens once at the first SIMD parameter.

  • Inserts an early return after emitting the realignment instruction.
  • Stops the loop to prevent redundant realignments for following SIMD params.
Comments suppressed due to low confidence (1)

src/mono/mono/mini/interp/transform.c:3398

  • Consider adding a unit test to verify that interp_realign_simd_params only performs realignment for the first SIMD parameter and skips subsequent parameters.
			return;

For untiered code, we don't use the var offset allocator, the offset for vars is computed on the fly as we push and pop them from the execution stack during initial IL import compilation phase. These offsets (and their alignment) follow the same rules as if they would be passed as arguments to a method. When we initially push a var we don't know if this var will be an argument to a call, so it might not have the stack alignment of 16 bytes. In this case, at the moment of a call we add a MINT_MOV_STACK_UNOPT opcode which moves the entire var space so it has the right alignment. The problem is that all simd params (which need 16byte alignment) will now become unaligned. `interp_realign_simd_params` is called to resolve this problem by doing another mov, this time starting at the location of the first simd argument.

This realignment is meant to be done only once, because once a simd argument is properly aligned, all following arguments are aligned as well, due to the way they are initially pushed on the execution stack. The issue was that we were not breaking the loop once we do the realignment starting at the first simd argument.
@BrzVlad BrzVlad force-pushed the fix-interp-simd-align branch from 2aba785 to ebc5730 Compare July 10, 2025 13:53
@lewing lewing enabled auto-merge (squash) July 10, 2025 19:00
@lewing lewing merged commit 135a4af into dotnet:main Jul 10, 2025
67 of 70 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

interpreter causes crash when using GKMeshGraph
3 participants