Skip to content

Conversation

@snickolls-arm
Copy link
Contributor

Includes:

  • ShiftRightAndInsert
  • ShiftRightArithmeticAdd
  • ShiftRightArithmeticNarrowingSaturateEven
  • ShiftRightArithmeticNarrowingSaturateOdd
  • ShiftRightArithmeticNarrowingSaturateUnsignedEven
  • ShiftRightArithmeticNarrowingSaturateUnsignedOdd
  • ShiftRightArithmeticRounded
  • ShiftRightArithmeticRoundedAdd
  • ShiftRightArithmeticRoundedNarrowingSaturateEven
  • ShiftRightArithmeticRoundedNarrowingSaturateOdd
  • ShiftRightArithmeticRoundedNarrowingSaturateUnsignedEven
  • ShiftRightArithmeticRoundedNarrowingSaturateUnsignedOdd
  • ShiftRightLogicalAdd
  • ShiftRightLogicalNarrowingEven
  • ShiftRightLogicalNarrowingOdd
  • ShiftRightLogicalRounded
  • ShiftRightLogicalRoundedAdd
  • ShiftRightLogicalRoundedNarrowingEven
  • ShiftRightLogicalRoundedNarrowingOdd
  • ShiftRightLogicalRoundedNarrowingSaturateOdd
  • ShiftRightLogicalRoundedNarrowingSaturateEven

Contributing towards #115479

@a74nh @kunalspathak

Includes:
* ShiftRightAndInsert
* ShiftRightArithmeticAdd
* ShiftRightArithmeticNarrowingSaturateEven
* ShiftRightArithmeticNarrowingSaturateOdd
* ShiftRightArithmeticNarrowingSaturateUnsignedEven
* ShiftRightArithmeticNarrowingSaturateUnsignedOdd
* ShiftRightArithmeticRounded
* ShiftRightArithmeticRoundedAdd
* ShiftRightArithmeticRoundedNarrowingSaturateEven
* ShiftRightArithmeticRoundedNarrowingSaturateOdd
* ShiftRightArithmeticRoundedNarrowingSaturateUnsignedEven
* ShiftRightArithmeticRoundedNarrowingSaturateUnsignedOdd
* ShiftRightLogicalAdd
* ShiftRightLogicalNarrowingEven
* ShiftRightLogicalNarrowingOdd
* ShiftRightLogicalRounded
* ShiftRightLogicalRoundedAdd
* ShiftRightLogicalRoundedNarrowingEven
* ShiftRightLogicalRoundedNarrowingOdd
* ShiftRightLogicalRoundedNarrowingSaturateOdd
* ShiftRightLogicalRoundedNarrowingSaturateEven
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 23, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics
See info in area-owners.md if you want to be subscribed.


/// <summary>
/// svint16_t svqrshrnt[_n_s32](svint16_t even, svint32_t op1, uint64_t imm2)
/// SQRSHRNT Ztied.H, Zop1.S, #imm2
Copy link
Contributor

Choose a reason for hiding this comment

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

SQRSHRNT

i don't think it is Ztied.H...should be just Zresult.H

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The destination register is used as a source here because this instruction only writes the odd numbered elements and leaves the even elements unchanged. The ACLE intrinsic has the even parameter which is the initial state of the destination register.

public static Vector<byte> ShiftRightAndInsert(Vector<byte> left, Vector<byte> right, [ConstantExpected] byte shift) => ShiftRightAndInsert(left, right, shift);

/// <summary>
/// svint16_t svsri[_n_s16](svint16_t op1, svint16_t op2, uint64_t imm3)
Copy link
Contributor

Choose a reason for hiding this comment

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

svsri

the parameter name left doesn't make sense here. left acts as a destination vector where results are merged. When I see left and right, I expect some kind of binary operation done by API, which is definitely not the case here. We should take this to API review committee.

HARDWARE_INTRINSIC(Sve2, ShiftRightArithmeticRoundedNarrowingSaturateUnsignedEven, -1, 2, {INS_invalid, INS_sve_sqrshrunb, INS_invalid, INS_sve_sqrshrunb, INS_invalid, INS_sve_sqrshrunb, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_ShiftRightByImmediate, HW_Flag_Scalable|HW_Flag_HasImmediateOperand)
HARDWARE_INTRINSIC(Sve2, ShiftRightArithmeticRoundedNarrowingSaturateUnsignedOdd, -1, 3, {INS_invalid, INS_sve_sqrshrunt, INS_invalid, INS_sve_sqrshrunt, INS_invalid, INS_sve_sqrshrunt, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_ShiftRightByImmediate, HW_Flag_Scalable|HW_Flag_HasImmediateOperand|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve2, ShiftRightLogicalAdd, -1, 3, {INS_invalid, INS_sve_usra, INS_invalid, INS_sve_usra, INS_invalid, INS_sve_usra, INS_invalid, INS_sve_usra, INS_invalid, INS_invalid}, HW_Category_ShiftRightByImmediate, HW_Flag_Scalable|HW_Flag_HasImmediateOperand|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve2, ShiftRightLogicalNarrowingEven, -1, 2, {INS_sve_shrnb, INS_sve_shrnb, INS_sve_shrnb, INS_sve_shrnb, INS_sve_shrnb, INS_sve_shrnb, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_ShiftRightByImmediate, HW_Flag_Scalable|HW_Flag_HasImmediateOperand)
Copy link
Contributor

Choose a reason for hiding this comment

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

shrnb just operates on unsigned integer value. @a74nh - do you know why we are exposing ShiftRightLogicalNarrowingEven for signed and unsigned? Similar argument goes for other APIs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Signed and unsigned versions exist in the ACLE: https://developer.arm.com/architectures/instruction-sets/intrinsics/#f:@navigationhierarchiessimdisa=[sve2]&q=svshrnb

I don't think the signedness of the input really matters, just that everything gets shifted right and narrowed.

If we just had one version, I can imagine it creating lots of casting when used with the wrong type. Similar to how were looking at adding additional type for SVE APIs we did last year.

Copy link
Contributor

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

added some questions about signed/unsigned values of APIs

Copy link
Contributor

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM. @a74nh just confirm about #116914 (comment)

@kunalspathak
Copy link
Contributor

/ba-g failures unrelated

@kunalspathak kunalspathak merged commit 3224c6c into dotnet:main Jun 25, 2025
151 of 160 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Runtime.Intrinsics 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.

3 participants