Skip to content

[release/8.0-preview3] [mono][jit] Increased max length of OP_STOREX_MEMBASE to 16 on arm64. - #83887

Merged
carlossanlop merged 1 commit into
release/8.0-preview3from
backport/pr-83770-to-release/8.0-preview3
Mar 24, 2023
Merged

[release/8.0-preview3] [mono][jit] Increased max length of OP_STOREX_MEMBASE to 16 on arm64.#83887
carlossanlop merged 1 commit into
release/8.0-preview3from
backport/pr-83770-to-release/8.0-preview3

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

Backport of #83770 to release/8.0-preview3

/cc @jandupej

Customer Impact

Crashes can occur on arm64 platforms (JIT) when SIMD is used through Vector128 operators. This resolves the crashes by fixing the expected maximum codegen length for OP_STOREX_MEMBASE.

Testing

The failures manifest in CI on arm64 platforms in System.Numerics.Vectors.Tests. Assertions like wrong maximal instruction length of instruction storex_membase (expected 12, got 16) Assertion: should not be reached at /__w/1/s/src/mono/mono/mini/mini-arm64.c:5066 are generated. The fix removes those failures.

Risk

Low.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@lambdageeklambdageek added the Servicing-consider Issue for next servicing release review label Mar 24, 2023
@lambdageeklambdageek added this to the 8.0.0 milestone Mar 24, 2023
@lambdageek

Copy link
Copy Markdown
Member

cc @carlossanlop we'd like to get this one in too, if there's still time.

@carlossanlop

carlossanlop commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

@mmitche do we have runway? This one's important.

@mmitche

Copy link
Copy Markdown
Member

Yes.

@jandupej

Copy link
Copy Markdown
Contributor

CI failures are unrelated.

@SamMonoRT

Copy link
Copy Markdown
Member

/cc @carlossanlop@mmitche - CI failures are unrelated. This is ready to be merged pending Tactics approval.

@carlossanlop

Copy link
Copy Markdown
Contributor

Approved by Tactics via email.

@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 24, 2023
@carlossanlop
carlossanlop merged commit 00a1a9a into release/8.0-preview3Mar 24, 2023
@carlossanlop
carlossanlop deleted the backport/pr-83770-to-release/8.0-preview3 branch March 24, 2023 19:19
@ghostghost locked as resolved and limited conversation to collaborators Apr 24, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-meta-monoServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@lambdageek@carlossanlop@mmitche@jandupej@SamMonoRT@vargaz