Skip to content

Conversation

@TIHan
Copy link
Contributor

@TIHan TIHan commented Aug 4, 2023

Fixes #89868

The vector right-shift instructions do not allow a zero immediate value in the encoding. See the docs.

shift
Is the right shift amount, in the range 1 to the element width in bits, and can be one of the values shown in Usage.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 4, 2023
@ghost ghost assigned TIHan Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

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

Issue Details

Fixes #89868

The vector right-shift instructions do not allow a zero immediate value in the encoding. See the docs.

shift
Is the right shift amount, in the range 1 to the element width in bits, and can be one of the values shown in Usage.

Author: TIHan
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@TIHan TIHan marked this pull request as ready for review August 7, 2023 02:12
GetEmitter()->emitIns_R_R_I(ins, emitSize, targetReg, op2Reg, shiftAmount, opt);
if (shiftAmount == 0)
{
GetEmitter()->emitIns_R_R_R(INS_mov, emitTypeSize(node), targetReg, reg, reg);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We really want to use emitIns_Mov but there is an issue with the ARM64 impl that will elide the mov even if canSkip is false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just checked the X64 impl and it has a similar issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is eliding a mov an issue here?

Copy link
Member

Choose a reason for hiding this comment

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

It breaks the jump table logic which expects a fixed number of bytes per switch case (on Arm64 that's 2 instructions per case, so 8-bytes)

@TIHan
Copy link
Contributor Author

TIHan commented Aug 8, 2023

@dotnet/jit-contrib @BruceForstall this is ready.

@BruceForstall BruceForstall merged commit 32cf5df into dotnet:main Aug 9, 2023
@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.0 milestone Aug 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2023
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.

Incorrect codegen on Arm64

5 participants