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

Don't return 0 from getMaxVectorByteLength when intrinsics are disabled #87420

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

tannergooding
Copy link
Member

This works around the issues tracked by:

These represent existing issues that were exposed by #85551 as it started having getMaxVectorByteLength return 0 if the relevant ISAs were disabled and therefore caused the JIT to go down its fallback paths (i.e. the path that might be executed on x86 Unix or Arm32).

The proper fixes for these are likely going to be a bit more involved and may require distinguishing what exists as required ABI handling (and therefore must use SIMD) vs what only exists as a perf optimization and therefore needs its fallback path to operate correctly even if the ISA is not available.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 12, 2023
@ghost ghost assigned tannergooding Jun 12, 2023
@ghost
Copy link

ghost commented Jun 12, 2023

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

Issue Details

This works around the issues tracked by:

These represent existing issues that were exposed by #85551 as it started having getMaxVectorByteLength return 0 if the relevant ISAs were disabled and therefore caused the JIT to go down its fallback paths (i.e. the path that might be executed on x86 Unix or Arm32).

The proper fixes for these are likely going to be a bit more involved and may require distinguishing what exists as required ABI handling (and therefore must use SIMD) vs what only exists as a perf optimization and therefore needs its fallback path to operate correctly even if the ISA is not available.

Author: tannergooding
Assignees: tannergooding
Labels:

area-CodeGen-coreclr

Milestone: -

@tannergooding
Copy link
Member Author

/azp run runtime-coreclr jitstress-isas-x86, runtime-coreclr jitstress-isas-arm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@tannergooding tannergooding marked this pull request as ready for review June 13, 2023 17:23
@tannergooding
Copy link
Member Author

CC. @dotnet/jit-contrib. This resolves all the jitstress-isas-x86 failures. There remain some unrelated jitstress-isas-arm failures.

As per the top post, this is namely a workaround and puts us back to our previous behavior. There are some underlying pre-existing issues that would be good to resolve longer term.

@EgorBo
Copy link
Member

EgorBo commented Jun 13, 2023

I assume it happens because we have all the SIMD stuff that can be disabled by EnableXXX/HWIntrinsics, but also, we have direct SIMD usages where we just assume that the baseline simd is always around?

@tannergooding
Copy link
Member Author

I assume it happens because we have all the SIMD stuff that can be disabled by EnableXXX/HWIntrinsics, but also, we have direct SIMD usages where we just assume that the baseline simd is always around

I believe part of it is that, yes. But I think more-so there is ABI handling that is split between the two today and it should instead live exclusively on one or the other.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants