Skip to content

[S390X] Add simd fallback support for unsupported s390x architectures - #118376

Merged
akoeplinger merged 4 commits into
dotnet:mainfrom
saitama951:add_fallback
Aug 18, 2025
Merged

[S390X] Add simd fallback support for unsupported s390x architectures#118376
akoeplinger merged 4 commits into
dotnet:mainfrom
saitama951:add_fallback

Conversation

@saitama951

@saitama951saitama951 commented Aug 5, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Vector facility was introduced in the z13, which majorly introduces vector int/short/long/double variants of the vector instructions. with the release of z14 we introduced vector float variant and vector max/min as part of vector enhancement facility 1.
with my previous patches #116779, #116669
we have already added support to z14 and above generations.

this patch supports z13 and previous generations

@github-actionsgithub-actionsBot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 5, 2025
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Aug 5, 2025
@risc-vv

risc-vv commented Aug 5, 2025

Copy link
Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.

kasperk81
kasperk81 reviewed Aug 5, 2025
View reviewed changes

@kasperk81kasperk81 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tab indentation

Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
@saitama951

saitama951 commented Aug 5, 2025

Copy link
Copy Markdown
ContributorAuthor

@uweigand can you please review this?

@saitama951
saitama951force-pushed the add_fallback branch 2 times, most recently from 159b67b to 96e621bCompareAugust 5, 2025 09:11
@saitama951
[S390X] Add simd fallback support for unsupported s390x architectures
97f2fb6
Vector facility was introduced in the z13, which majorly
introduces vector int/short/long/double variants of the
vector instructions. with the release of z14 we introduced
vector float variant as part of vector enhancement facility 1.
This patch majorly supports z13 and previous generations.
@saitama951@kasperk81
Apply suggestions from code review
f5e4c47
fix code style
Co-authored-by: kasperk81 <83082615+kasperk81@users.noreply.github.com>
@jkotasjkotas added area-Codegen-JIT-mono arch-wasm WebAssembly architecture arch-s390x Related to s390x architecture (unsupported) and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners arch-wasm WebAssembly architecture labels Aug 5, 2025
@dotnet-policy-service

dotnet-policy-serviceBot commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @vitek-karas
See info in area-owners.md if you want to be subscribed.

This was referenced Aug 5, 2025
Open
Open
Open
Open
@saitama951
fix errors
579eda9
* fix test case failures on arm64
* fix more code style reviews
@build-analysisbuild-analysisBot mentioned this pull request Aug 6, 2025
Closed
uweigand
uweigand reviewed Aug 8, 2025
View reviewed changes

@uweiganduweigand left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In addition to the inline comments, you should also fix this:

#define MONO_ARCH_SIMD_INTRINSICS mono_hwcap_s390x_has_vec

and define it to just 1 like everywhere else, since this is not actually evaluated at run-time.

Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
Comment threadsrc/mono/mono/mini/simd-intrinsics.c
Comment threadsrc/mono/mono/mini/simd-intrinsics.c Outdated
saitama951
saitama951 commented Aug 14, 2025
View reviewed changes
Comment threadsrc/mono/mono/mini/mini.h
@build-analysisbuild-analysisBot mentioned this pull request Aug 14, 2025
Open
uweigand
uweigand reviewed Aug 15, 2025
View reviewed changes

@uweiganduweigand left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This version looks good to me now from a s390x target perspective.

@saitama951

saitama951 commented Aug 16, 2025

Copy link
Copy Markdown
ContributorAuthor

@akoeplinger can this be merged?

@akoeplinger

akoeplinger commented Aug 18, 2025

Copy link
Copy Markdown
Member

/ba-g failures are unrelated to s390x changes

@akoeplinger
akoeplinger merged commit 4e7ba24 into dotnet:mainAug 18, 2025
74 of 80 checks passed
@saitama951

saitama951 commented Aug 18, 2025

Copy link
Copy Markdown
ContributorAuthor

@akoeplinger can we backport this to release/10.0 as well?

@akoeplinger

akoeplinger commented Aug 18, 2025

Copy link
Copy Markdown
Member

@lewing do we have a policy for changes which only touch community-supported arch code?

@saitama951

saitama951 commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

@lewing gentle ping.

@lewing do we have a policy for changes which only touch community-supported arch code?

@giritrivedi

giritrivedi commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

@akoeplinger , @lewing - Can we have this backported ? or is there a policy which restricts us from backporting ?

@lewing

lewing commented Sep 2, 2025

Copy link
Copy Markdown
Member

/backport to release/10.0

@github-actions

github-actionsBot commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17413301220

@github-actionsgithub-actionsBot mentioned this pull request Sep 2, 2025
Merged
4 tasks
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Oct 3, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplingerakoeplingerakoeplinger approved these changes
@steveisoksteveisokAwaiting requested review from steveisoksteveisok is a code owner
@vitek-karasvitek-karasAwaiting requested review from vitek-karasvitek-karas is a code owner
+2 more reviewers
@uweiganduweiganduweigand left review comments
@kasperk81kasperk81kasperk81 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

arch-s390xRelated to s390x architecture (unsupported)area-Codegen-JIT-monocommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@saitama951@risc-vv@akoeplinger@giritrivedi@lewing@uweigand@kasperk81@jkotas