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.

Sign up for GitHub

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

JIT: Added four SVE CreateBreak* APIs #104184

Merged
merged 6 commits into from
Jul 2, 2024
+620 −0
Merged

JIT: Added four SVE CreateBreak* APIs #104184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5409c86
Added four SVE CreateBreak* APIs
TIHan Jun 29, 2024
a18d66b
Merging
TIHan Jul 1, 2024
c077d1a
Fix comments
TIHan Jul 1, 2024
1cde299
Added break
TIHan Jul 1, 2024
790882e
Formatting
TIHan Jul 1, 2024
c7958cc
Update Sve.PlatformNotSupported.cs
TIHan Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/coreclr/jit/hwintrinsic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,10 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,

switch (intrinsic)
{
case NI_Sve_CreateBreakAfterMask:
case NI_Sve_CreateBreakAfterPropagateMask:
case NI_Sve_CreateBreakBeforeMask:
case NI_Sve_CreateBreakBeforePropagateMask:
case NI_Sve_CreateMaskForFirstActiveElement:
case NI_Sve_CreateMaskForNextActiveElement:
case NI_Sve_GetActiveElementCount:
Expand All @@ -1933,6 +1937,26 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
break;
}

switch (intrinsic)
{
case NI_Sve_CreateBreakAfterPropagateMask:
case NI_Sve_CreateBreakBeforePropagateMask:
{
GenTree* op3 = retNode->AsHWIntrinsic()->Op(3);

// HWInstrinsic requires a mask for op3
if (!varTypeIsMask(op3))
{
retNode->AsHWIntrinsic()->Op(3) =
gtNewSimdCvtVectorToMaskNode(TYP_MASK, op3, simdBaseJitType, simdSize);
}
break;
}
TIHan marked this conversation as resolved.
Show resolved Hide resolved

default:
break;
}

if (!varTypeIsMask(op1))
{
// Op1 input is a vector. HWInstrinsic requires a mask.
Expand Down
14 changes: 14 additions & 0 deletions src/coreclr/jit/hwintrinsiccodegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2238,6 +2238,20 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
break;
}

case NI_Sve_CreateBreakAfterMask:
case NI_Sve_CreateBreakBeforeMask:
{
GetEmitter()->emitInsSve_R_R_R(ins, emitSize, targetReg, op1Reg, op2Reg, INS_OPTS_SCALABLE_B);
Copy link
Member

Choose a reason for hiding this comment

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

For BRKB and BRKA, if we do not pass insScalableOpts, it is treated as BRK* Pd.B, Pg/Z, Pn.B. How do we generate the merge variant? If we want to support one of those, should merge be the default one? Accordingly we should also update the summary docs and remove the instruction that we are not supporting. We also need to think about what if all targetReg, op1Reg and op2Reg end up to same register or two of them have same register, does it change the operation of Pg/M vs. Pg/Z. Lastly, we should add test coverage (if not there currently) where we pass same value to mask and srcMask to simulate such behavior.

Copy link
Contributor Author

@TIHan TIHan Jun 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When looking at the original generated API, there were two C intrinsic variants that it listed:
M variant https://dougallj.github.io/asil/doc/brka_p_p_p_m_8.html
Z variant https://dougallj.github.io/asil/doc/brka_p_p_p_z_8.html

Based on the API signature of our intrinsic, it matches exactly to the Z variant - so naturally I think that's most likely the correct one. I figured the M variant that was listed is an artifact of the API generation; as other APIs have before. This also leads me to believe that exposing each variant needs to have their own API.

Copy link
Contributor

Choose a reason for hiding this comment

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

I figured the M variant that was listed is an artifact of the API generation

Yes, the API generation will have just blindly put them together.

This also leads me to believe that exposing each variant needs to have their own API.

We can we replicate the zero version using:

CndSel(mask, CreateBreakAfterMask(mask, op1), zero)

And then in lowering or somewhere, optimise that to a single BRKA with zeroing.

(Or, if I'm wrong and that doesn't work, then yes, ideally add a extra API)

Doesn't need doing now, but could you raise a ticket please.

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 can we replicate the zero version using:

Do you mean "merge"? The API is the zero version currently

Copy link
Member

Choose a reason for hiding this comment

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

could you raise a ticket please.

+1 on that if we decide to skip a variant of the instruction.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with just keeping "Z" version. Just update the summary docs.

https://github.com/dotnet/runtime/pull/104184/files#diff-65452cc3b51792a27a23d428011052b18c0b7de4576424a195e0cf5003dc6a9cR987-R988

break;
}

case NI_Sve_CreateBreakAfterPropagateMask:
case NI_Sve_CreateBreakBeforePropagateMask:
{
GetEmitter()->emitInsSve_R_R_R_R(ins, emitSize, targetReg, op1Reg, op2Reg, op3Reg, INS_OPTS_SCALABLE_B);
break;
}

case NI_Sve_CreateMaskForFirstActiveElement:
{
assert(isRMW);
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/hwintrinsiclistarm64sve.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ HARDWARE_INTRINSIC(Sve, Count16BitElements,
HARDWARE_INTRINSIC(Sve, Count32BitElements, 0, 1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_cntw, INS_invalid, INS_invalid, INS_invalid}, HW_Category_Scalar, HW_Flag_Scalable|HW_Flag_HasEnumOperand|HW_Flag_SpecialCodeGen|HW_Flag_NoFloatingPointUsed)
HARDWARE_INTRINSIC(Sve, Count64BitElements, 0, 1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_cntd, INS_invalid, INS_invalid, INS_invalid}, HW_Category_Scalar, HW_Flag_Scalable|HW_Flag_HasEnumOperand|HW_Flag_SpecialCodeGen|HW_Flag_NoFloatingPointUsed)
HARDWARE_INTRINSIC(Sve, Count8BitElements, 0, 1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_cntb, INS_invalid, INS_invalid, INS_invalid}, HW_Category_Scalar, HW_Flag_Scalable|HW_Flag_HasEnumOperand|HW_Flag_SpecialCodeGen|HW_Flag_NoFloatingPointUsed)
HARDWARE_INTRINSIC(Sve, CreateBreakAfterMask, -1, 2, true, {INS_sve_brka, INS_sve_brka, INS_sve_brka, INS_sve_brka, INS_sve_brka, INS_sve_brka, INS_sve_brka, INS_sve_brka, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_ReturnsPerElementMask|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Sve, CreateBreakAfterPropagateMask, -1, 3, true, {INS_sve_brkpa, INS_sve_brkpa, INS_sve_brkpa, INS_sve_brkpa, INS_sve_brkpa, INS_sve_brkpa, INS_sve_brkpa, INS_sve_brkpa, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_ReturnsPerElementMask|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Sve, CreateBreakBeforeMask, -1, 2, true, {INS_sve_brkb, INS_sve_brkb, INS_sve_brkb, INS_sve_brkb, INS_sve_brkb, INS_sve_brkb, INS_sve_brkb, INS_sve_brkb, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_ReturnsPerElementMask|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Sve, CreateBreakBeforePropagateMask, -1, 3, true, {INS_sve_brkpb, INS_sve_brkpb, INS_sve_brkpb, INS_sve_brkpb, INS_sve_brkpb, INS_sve_brkpb, INS_sve_brkpb, INS_sve_brkpb, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_ReturnsPerElementMask|HW_Flag_SpecialCodeGen)
Copy link
Member

Choose a reason for hiding this comment

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

wondering why you did not include CreateBreakPropagateMask?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that API needs a closer look as its signature doesn't correspond well to its instruction based on its parameter names and parameter count.

HARDWARE_INTRINSIC(Sve, CreateFalseMaskByte, -1, 0, false, {INS_invalid, INS_sve_pfalse, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ReturnsPerElementMask)
HARDWARE_INTRINSIC(Sve, CreateFalseMaskDouble, -1, 0, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_pfalse}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ReturnsPerElementMask)
HARDWARE_INTRINSIC(Sve, CreateFalseMaskInt16, -1, 0, false, {INS_invalid, INS_invalid, INS_sve_pfalse, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ReturnsPerElementMask)
Expand Down
193 changes: 193 additions & 0 deletions ...ries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Arm/Sve.PlatformNotSupported.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,199 @@ internal Arm64() { }
public static unsafe ulong Count8BitElements([ConstantExpected] SveMaskPattern pattern = SveMaskPattern.All) { throw new PlatformNotSupportedException(); }


/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<byte> CreateBreakAfterMask(Vector<byte> mask, Vector<byte> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<short> CreateBreakAfterMask(Vector<short> mask, Vector<short> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<int> CreateBreakAfterMask(Vector<int> mask, Vector<int> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<long> CreateBreakAfterMask(Vector<long> mask, Vector<long> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<sbyte> CreateBreakAfterMask(Vector<sbyte> mask, Vector<sbyte> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<ushort> CreateBreakAfterMask(Vector<ushort> mask, Vector<ushort> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<uint> CreateBreakAfterMask(Vector<uint> mask, Vector<uint> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrka[_b]_z(svbool_t pg, svbool_t op)
/// BRKA Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<ulong> CreateBreakAfterMask(Vector<ulong> mask, Vector<ulong> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<byte> CreateBreakAfterPropagateMask(Vector<byte> mask, Vector<byte> left, Vector<byte> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<short> CreateBreakAfterPropagateMask(Vector<short> mask, Vector<short> left, Vector<short> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<int> CreateBreakAfterPropagateMask(Vector<int> mask, Vector<int> left, Vector<int> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<long> CreateBreakAfterPropagateMask(Vector<long> mask, Vector<long> left, Vector<long> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<sbyte> CreateBreakAfterPropagateMask(Vector<sbyte> mask, Vector<sbyte> left, Vector<sbyte> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<ushort> CreateBreakAfterPropagateMask(Vector<ushort> mask, Vector<ushort> left, Vector<ushort> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<uint> CreateBreakAfterPropagateMask(Vector<uint> mask, Vector<uint> left, Vector<uint> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpa[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPA Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<ulong> CreateBreakAfterPropagateMask(Vector<ulong> mask, Vector<ulong> left, Vector<ulong> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<byte> CreateBreakBeforeMask(Vector<byte> mask, Vector<byte> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<short> CreateBreakBeforeMask(Vector<short> mask, Vector<short> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<int> CreateBreakBeforeMask(Vector<int> mask, Vector<int> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<long> CreateBreakBeforeMask(Vector<long> mask, Vector<long> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<sbyte> CreateBreakBeforeMask(Vector<sbyte> mask, Vector<sbyte> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<ushort> CreateBreakBeforeMask(Vector<ushort> mask, Vector<ushort> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<uint> CreateBreakBeforeMask(Vector<uint> mask, Vector<uint> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkb[_b]_z(svbool_t pg, svbool_t op)
/// BRKB Presult.B, Pg/Z, Pop.B
/// </summary>
public static unsafe Vector<ulong> CreateBreakBeforeMask(Vector<ulong> mask, Vector<ulong> srcMask) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<byte> CreateBreakBeforePropagateMask(Vector<byte> mask, Vector<byte> left, Vector<byte> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<short> CreateBreakBeforePropagateMask(Vector<short> mask, Vector<short> left, Vector<short> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<int> CreateBreakBeforePropagateMask(Vector<int> mask, Vector<int> left, Vector<int> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<long> CreateBreakBeforePropagateMask(Vector<long> mask, Vector<long> left, Vector<long> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<sbyte> CreateBreakBeforePropagateMask(Vector<sbyte> mask, Vector<sbyte> left, Vector<sbyte> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<ushort> CreateBreakBeforePropagateMask(Vector<ushort> mask, Vector<ushort> left, Vector<ushort> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<uint> CreateBreakBeforePropagateMask(Vector<uint> mask, Vector<uint> left, Vector<uint> right) { throw new PlatformNotSupportedException(); }

/// <summary>
/// svbool_t svbrkpb[_b]_z(svbool_t pg, svbool_t op1, svbool_t op2)
/// BRKPB Presult.B, Pg/Z, Pop1.B, Pop2.B
/// </summary>
public static unsafe Vector<ulong> CreateBreakBeforePropagateMask(Vector<ulong> mask, Vector<ulong> left, Vector<ulong> right) { throw new PlatformNotSupportedException(); }


/// Set all predicate elements to false

/// <summary>
Expand Down
Loading
Loading