Skip to content

Conversation

jonathandavies-arm
Copy link
Contributor

  • Added a struct for the test groups and templates
  • Moved Isa & LoadIsa values from each test to the test group
  • Tidied up the Proccess* functions

@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 Aug 5, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 5, 2025
@jkotas jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 5, 2025
Copy link
Contributor

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

@a74nh
Copy link
Contributor

a74nh commented Aug 11, 2025

@dotnet/arm64-contrib @amanasifkhalid

This is cleaner than the existing code, but it's a lot of code churn. The templates are just a each a one liner, then the same two column changes in all the *Tests.cs files and the proper code changes in the generator.

Because of the mechanical nature, and it's only the testing, this should be fairly low risk. But it's still a lot of code. I wonder if this should be left to .NET11?

@amanasifkhalid
Copy link
Contributor

Because of the mechanical nature, and it's only the testing, this should be fairly low risk. But it's still a lot of code. I wonder if this should be left to .NET11?

Unless anyone else has cold feet, I'm fine with taking this in .NET 10. @jonathandavies-arm could you PTAL at the merge conflicts? Once we get a CI run in, it should be easy to see if this regresses anything.

@a74nh
Copy link
Contributor

a74nh commented Aug 13, 2025

Because of the mechanical nature, and it's only the testing, this should be fairly low risk. But it's still a lot of code. I wonder if this should be left to .NET11?

Unless anyone else has cold feet, I'm fine with taking this in .NET 10. @jonathandavies-arm could you PTAL at the merge conflicts? Once we get a CI run in, it should be easy to see if this regresses anything.

Rebasing this is a pain and also is a pain for anyone with WIP to rebase on top. Given we're hitting .NET10 window, I suggest in order we:

  1. Get SVE2 FP API's #118332 merged
  2. @jacob-crawley raises a new PR to implement the missing FP APIs
  3. That'll take us more or less up to the August 15th deadline
  4. @jonathandavies-arm can rebase this PR, and then get it merged
  5. @jacob-crawley will be looking at scatterstores/gatherloads, which can go last

@amanasifkhalid
Copy link
Contributor

Rebasing this is a pain and also is a pain for anyone with WIP to rebase on top. Given we're hitting .NET10 window, I suggest in order we:

Agreed, sounds like a good plan

@jonathandavies-arm jonathandavies-arm force-pushed the upstream/refactor-GenerateHWIntrinsicTests_Arm branch from eb44ef1 to 1f8e372 Compare August 18, 2025 11:59
@jonathandavies-arm
Copy link
Contributor Author

Sorry for the force push but merging was confusing.

@JulieLeeMSFT JulieLeeMSFT added the arm-sve Work related to arm64 SVE/SVE2 support label Sep 29, 2025
@JulieLeeMSFT
Copy link
Member

@jonathandavies-arm, do you need help with resolving conflicts?

Sorry for the force push but merging was confusing.

@JulieLeeMSFT
Copy link
Member

#120291 unblocks this PR.
@jonathandavies-arm, please resolve conflicts and ping us when review is ready.

@jonathandavies-arm
Copy link
Contributor Author

#120291 unblocks this PR. @jonathandavies-arm, please resolve conflicts and ping us when review is ready.

@JulieLeeMSFT #118957 is also waiting to be merged and changes Sve2Tests.cs. I will start resolving the conflicts but I'll wait for this other PR before pushing it.

@JulieLeeMSFT
Copy link
Member

@JulieLeeMSFT #118957 is also waiting to be merged and changes Sve2Tests.cs. I will start resolving the conflicts but I'll wait for this other PR before pushing it.

Thanks for pointing it out. Asked Tanner to review #118957. Is it all you need?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI arm-sve Work related to arm64 SVE/SVE2 support community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants