Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arm64/Sve: Missing test coverage for invalid immediate #104809

Closed
kunalspathak opened this issue Jul 12, 2024 · 5 comments · Fixed by #105677
Closed

Arm64/Sve: Missing test coverage for invalid immediate #104809

kunalspathak opened this issue Jul 12, 2024 · 5 comments · Fixed by #105677
Assignees
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 in-pr There is an active PR which will close this issue when it is merged Priority:2 Work that is important, but not critical for the release
Milestone

Comments

@kunalspathak
Copy link
Member

kunalspathak commented Jul 12, 2024

All the test templates that verify the invalid immediate, is missing this piece of code. Thanks @amanasifkhalid for bringing this to my attention in #104697 (comment).

if (!succeeded)
{
    Succeeded = false;
}

There are also certain templates like _SveImmBinaryOpTestTemplate.template that doesn't even have this test method.

  • Need to audit all the *Imm*.template files that were added/updated as part of SVE work and make sure we add test coverage for it.
  • Also make sure that we have coverage for following:
    • When we use valid value via constant as the input using {Imm}
    • When we use valid value via variable as the input using Imm
    • When we use invalid valid via constant as the input and we throw ArgumentOutOfRangeException
    • When we use invalid value via variable as the input and we throw ArgumentOutOfRangeException
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jul 12, 2024
@kunalspathak
Copy link
Member Author

Assigning to @TIHan for now and we will re-distribute it if needed.

@kunalspathak
Copy link
Member Author

@dotnet/arm64-contrib

@jeffschwMSFT jeffschwMSFT added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 15, 2024
Copy link
Contributor

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

@JulieLeeMSFT JulieLeeMSFT removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 17, 2024
@JulieLeeMSFT JulieLeeMSFT added this to the 9.0.0 milestone Jul 17, 2024
@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 17, 2024
@JulieLeeMSFT JulieLeeMSFT added the arm-sve Work related to arm64 SVE/SVE2 support label Jul 25, 2024
@a74nh a74nh added the Priority:2 Work that is important, but not critical for the release label Jul 30, 2024
@a74nh
Copy link
Contributor

a74nh commented Jul 30, 2024

priority:2 for RC1 snap : Missing testing

@a74nh
Copy link
Contributor

a74nh commented Jul 31, 2024

Assign to myself because Mikhail is working on it.

@JulieLeeMSFT JulieLeeMSFT added the in-pr There is an active PR which will close this issue when it is merged label Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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 in-pr There is an active PR which will close this issue when it is merged Priority:2 Work that is important, but not critical for the release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants