Skip to content

Conversation

@kunalspathak
Copy link
Contributor

Backport of #107036 to release/9.0

Customer Impact

  • Customer reported
  • Found internally

This bug was found internally where we are trying to assign same register to source and destination for a SVE instruction that is prefixed by MOVPRFX instruction. As per spec, that is not allowed.

Regression

  • Yes
  • No

This is not a regression, but merely a newly discovered bug from our internal testing.

Testing

Added test cases that uncovered the problem and ran all the existing SVE tests under stress.

Risk

Low. This only affects certain SVE instructions that could have MOVPRFX as the prefix instruction and the fix avoid assign same register to the operands. Besides, SVE is an experimental feature for .NET 9.

* ARM64-SVE: Delay free all ops within conditional select

* Fix comment

* Add test header

* don't delay prefUseOpNum

* Fix FMA

* Add assert checks for delay free

* Merge embedded op build code

* fix formatting

* simplify assert

* simplify FMA code

* Add tests for 106867

* ARM64-SVE: Allow op inside conditionselect to be non HWintrinsic

TEST_IMG: ubuntu/dotnet-build
TEST_CMD: safe ./projects/dotnet/build-runtime.sh

Jira: ENTLLT-7634
Change-Id: I337a291be6661f104fe90c7cdc27150eede43647

* Add Sve.IsSupported to tests

* Add Sve.IsSupported to test

* fix formatting

* Revert "ARM64-SVE: Allow op inside conditionselect to be non HWintrinsic"

* Revert "ARM64-SVE: Allow op inside conditionselect to be non HWintrinsic"

* Revert "ARM64-SVE: Allow op inside conditionselect to be non HWintrinsic"
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 5, 2024
@dotnet-policy-service
Copy link
Contributor

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

@kunalspathak kunalspathak added the Servicing-consider Issue for next servicing release review label Sep 6, 2024
@kunalspathak kunalspathak requested a review from a74nh September 6, 2024 04:05
Copy link
Contributor

@a74nh a74nh left a comment

Choose a reason for hiding this comment

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

Agreed, this needs adding. Using the same argument multiple times in a call could quite easily happen in customer code using SVE.

Copy link
Member

@JulieLeeMSFT JulieLeeMSFT left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. we can merge when ready

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 6, 2024
@kunalspathak
Copy link
Contributor Author

@carlossanlop, @ViktorHofer @ericstj - this is ready to merge.

@carlossanlop carlossanlop merged commit 3c622af into dotnet:release/9.0 Sep 6, 2024
@ericstj
Copy link
Member

ericstj commented Sep 6, 2024

FYI - You can also ask @jeffschwMSFT to merge

@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 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 Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants