Skip to content

Arm64/Sve: Fix a SVE issue and add CI leg for testing SVE with AltJit - #104998

Merged
kunalspathak merged 6 commits into
dotnet:mainfrom
kunalspathak:sve-pipeline
Jul 17, 2024
Merged

Arm64/Sve: Fix a SVE issue and add CI leg for testing SVE with AltJit#104998
kunalspathak merged 6 commits into
dotnet:mainfrom
kunalspathak:sve-pipeline

Conversation

@kunalspathak

@kunalspathakkunalspathak commented Jul 16, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

Address issue recently introduced in #104875 and discussed in #104964 (comment).

Also, added a CI leg to run SVE test with altjit. Discussion: #104964 (comment)

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 16, 2024
@dotnet-policy-service

dotnet-policy-serviceBot commented Jul 16, 2024

Copy link
Copy Markdown
Contributor

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

@kunalspathak

kunalspathak commented Jul 16, 2024

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr jitstress-isas-sve

@azure-pipelines

azure-pipelinesBot commented Jul 16, 2024

Copy link
Copy Markdown
No pipelines are associated with this pull request.

@kunalspathakkunalspathak mentioned this pull request Jul 16, 2024
Merged
@kunalspathakkunalspathak added the arm-sve Work related to arm64 SVE/SVE2 support label Jul 16, 2024
@kunalspathakkunalspathak mentioned this pull request Jul 16, 2024
Merged
@kunalspathak

kunalspathak commented Jul 16, 2024

Copy link
Copy Markdown
ContributorAuthor

@dotnet/arm64-contrib

amanasifkhalid
amanasifkhalid reviewed Jul 16, 2024
View reviewed changes

@amanasifkhalidamanasifkhalid 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.

Thanks for adding the new CI leg!

Comment threadsrc/coreclr/jit/gentree.cpp Outdated
TIHan
TIHan approved these changes Jul 17, 2024
View reviewed changes

@TIHanTIHan 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.

LGTM, this is a really really good idea to do - at least we can get some sort of coverage for the time being.

@amanasifkhalidamanasifkhalid mentioned this pull request Jul 17, 2024
Merged
tannergooding
tannergooding reviewed Jul 17, 2024
View reviewed changes
Comment threadeng/pipelines/common/templates/runtimes/run-test-job.yml Outdated
@kunalspathak
kunalspathak marked this pull request as ready for review July 17, 2024 02:02
@kunalspathak

kunalspathak commented Jul 17, 2024

Copy link
Copy Markdown
ContributorAuthor

@tannergooding - wondering if there is a way to just run the Sve tests on those legs instead of entire test suite?

This was referenced Jul 17, 2024
Closed
Closed
Closed
Open
Closed
Closed
Closed
@tannergooding

tannergooding commented Jul 17, 2024

Copy link
Copy Markdown
Member

wondering if there is a way to just run the Sve tests on those legs instead of entire test suite?

Potentially, but I don't think its worth doing so. As soon as we move to enable Vector to use it by default in .NET 10 we're going to want all code to compile and to do so with SVE instructions being emitted.

tannergooding
tannergooding reviewed Jul 17, 2024
View reviewed changes
Comment threadsrc/tests/Common/testenvironment.proj
<TestEnvironment Include="jitstressregs4_arm64_sve" JitStressRegs="4" AltJitName="clrjit_universal_arm64_x64.dll" AltJit="*" MaxVectorTBitWidth="128" />
<TestEnvironment Include="jitstressregs8_arm64_sve" JitStressRegs="8" AltJitName="clrjit_universal_arm64_x64.dll" AltJit="*" MaxVectorTBitWidth="128" />

<TestEnvironment Include="jitstress_isas_arm64_sve" AltJitName="clrjit_universal_arm64_x64.dll" AltJit="*" MaxVectorTBitWidth="128" /> <!-- Trigger Sve code paths-->

@tannergoodingtannergoodingJul 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will this cause problems on an Arm64 box where I believe it would be clrjit_universal_arm64_arm64.dll instead?

@tannergoodingtannergoodingJul 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm guessing "no" by default since its set to run only on Windows_x64 machines?

@kunalspathakkunalspathakJul 17, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

that's right...and that was the reason i did not have on linux/x64 either, otherwise would have to specify libclrjit_universal_arm64_x64.so

@build-analysisbuild-analysisBot mentioned this pull request Jul 17, 2024
Closed
3 tasks
@kunalspathak

kunalspathak commented Jul 17, 2024

Copy link
Copy Markdown
ContributorAuthor

/ba-g the specialized test is failing because of timeout

@kunalspathak
kunalspathak merged commit 513b503 into dotnet:mainJul 17, 2024
@kunalspathak
kunalspathak deleted the sve-pipeline branch July 17, 2024 23:33
@kunalspathak

kunalspathak commented Jul 17, 2024

Copy link
Copy Markdown
ContributorAuthor

/azp list

@kunalspathakkunalspathak mentioned this pull request Jul 18, 2024
Closed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 17, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tannergoodingtannergoodingtannergooding approved these changes
+2 more reviewers
@amanasifkhalidamanasifkhalidamanasifkhalid left review comments
@TIHanTIHanTIHan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@kunalspathakkunalspathak

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIarm-sveWork related to arm64 SVE/SVE2 support

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kunalspathak@tannergooding@TIHan@amanasifkhalid