arm64: Clean up SVE embedded masked codegen

[ylpoonlg]

This PR is the last part for #115508, with the following changes:

• Cleanup to hwintrinsiccodegenarm64.cpp:

  • Move the embedded masked block to a new function genEmbeddedMaskedHWIntrinsic.
  • Combine codepaths for different number of operand cases and centralize movprfx logic.

• Optimizations to movprfx usage in embedded masked operation codegen:

  • Replace predicated movprfx with unpredicated movprfx when mask is all-true. Unpredicated movprfx is generally preferred due to performance.
  • Allow zero falseOp to be contained when mask is not all-true so that zeroing predicated movprfx can be used.
  • Allow unary embedded masked ops to use movprfx. Unary embedded masked ops are not RMW instructions, but may also support movprfx, which require the target register to be delayed free in LSRA.

• Fixing the Sve HardwareIntrinsics tests ConditionalSelect_ZeroOp calls: The falseOp->IsVectorZero branch in the codegen was previously untested because the zero vector was passed as a local variable rather than a constant vector. The zero vector needs to be passed directly into the ConditionalSelect intrinsic in the test templates.

[dotnet-policy-service]

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

[ylpoonlg]

Example SPMI asmdiffs:

@@ -22,11 +22,11 @@
             movz    x0, #0xD1FFAB1E      // data for <unknown class>:<unknown field>
             movk    x0, #0xD1FFAB1E LSL #16
             movk    x0, #0xD1FFAB1E LSL #32
             ldr     q17, [x0]
             ptrue   p0.d
-            movprfx z18.d, p0/z, z16.d
+            movprfx z18, z16
             fmla    z18.d, p0/m, z17.d, z17.d
             str     q18, [x0]

Replaces predicated movprfx with unpredicated movprfx when predicate is true.

@@ -20,31 +20,30 @@
                                                ;; size=16 bbWeight=1 PerfScore 3.50
 G_M32226_IG02:        ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref
             ldr     q16, [fp, #0x20]   // [V00 arg0]
             ldr     q17, [fp, #0x10]   // [V01 arg1]
             ptrue   p0.b
-            movprfx z16.b, p0/z, z16.b
             smax    z16.b, p0/m, z16.b, z17.b
             mov     v0.16b, v16.16b

Removes unnecessary movprfx when moving the same register and predicate is all-true.

@@ -195,14 +191,12 @@
             blr     x1      // code for <unknown method>
             ; gcrRegs -[x0]
             ldp     q1, q3, [x19, #0x30]
             mov     v2.16b, v3.16b
             ptrue   p0.d
-            movi    v0.4s, #0
             movprfx z4, z1
             faddp   z4.d, p0/m, z4.d, z2.d
-            sel     z4.d, p0, z4.d, z0.d
             mvni    v0.4s, #0
             mov     x0, x19
             ; gcrRegs +[x0]
             mov     w1, #1
             mov     x2, x21

Removes unnecessary selects and contain zero vector when mask is all-true.

             ; gcrRegs -[x0]
             ldr     q0, [x19, #0x18]
             ldr     q1, [x19, #0x28]
             ptrue   p0.d
             cmpne   p0.d, p0/z, z0.d, #0
-            movi    v3.4s, #0
+            movprfx z3.d, p0/z, z3.d
             frintn  z3.d, p0/m, z1.d
             movi    v2.4s, #0
             mov     x0, x19
             ; gcrRegs +[x0]
             mov     w1, #1

Contains the zero vector and use zeroing movprfx.

@@ -150,13 +150,14 @@
             ptrue   p0.s
             cmpne   p0.s, p0/z, z8.s, #0
             movi    v16.4s, #0
             mov     z12.d, z9.d
             fcvtnt  z12.s, p0/m, z10.d
+            sel     z12.s, p0, z12.s, z16.s
             str     q12, [fp, #0x20]   // [V07 tmp6]
             b       G_M36822_IG05

Fixes a bug with inactive lanes for ConvertToSingleOdd. This was revealed by the changes to the test templates.

[ylpoonlg]

jitstressregs passes locally.

@dotnet/arm64-contrib @a74nh @dhartglassMSFT

[ylpoonlg]

error : (NETCORE_ENGINEERING_TELEMETRY=Test) Test JIT/opt/SVE/ConstantMasks/ConstantMasks.cmd has failed.

Some optimization tests are failing, I will look into them further.

[ylpoonlg]

There is still a failing SVE opt test ConstantMasks:

        552: G_M15649_IG02:
        553:  ptrue p0.s
        554:  abs z16.s, p0/m, z0.s
        555:  mov v0.16b, v16.16b
        556:  movz x0, #0xD1FFAB1E // code for ConstantMasks:Consume[System.Numerics.Vector`1[int]](System.Numerics.Vector`1[int])
next:179     !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  error: match on wrong line

The LSRA change caused an extra mov v0.16b, v16.16b, but I have not been able to prioritize the destination register over op1 (any ideas would be appreciated). This was done to allow for movprfx usage, but in this case the ptrue suppressed it, so it doesn't really need to be delayed free. Maybe I should revert to using SVE mov instead of movprfx for now, and leave this for #107134.

[dhartglassMSFT]

There is still a failing SVE opt test ConstantMasks:

        552: G_M15649_IG02:
        553:  ptrue p0.s
        554:  abs z16.s, p0/m, z0.s
        555:  mov v0.16b, v16.16b
        556:  movz x0, #0xD1FFAB1E // code for ConstantMasks:Consume[System.Numerics.Vector`1[int]](System.Numerics.Vector`1[int])
next:179     !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  error: match on wrong line

The LSRA change caused an extra mov v0.16b, v16.16b, but I have not been able to prioritize the destination register over op1 (any ideas would be appreciated). This was done to allow for movprfx usage, but in this case the ptrue suppressed it, so it doesn't really need to be delayed free. Maybe I should revert to using SVE mov instead of movprfx for now, and leave this for #107134.

Hey @ylpoonlg @a74nh following up on this. I may have lost some context, but during lsra we know whether the mask is all true, correct? In any case let me know if you needed further follow-up here, sorry for the delay

[ylpoonlg]

There is still a failing SVE opt test ConstantMasks:

        552: G_M15649_IG02:
        553:  ptrue p0.s
        554:  abs z16.s, p0/m, z0.s
        555:  mov v0.16b, v16.16b
        556:  movz x0, #0xD1FFAB1E // code for ConstantMasks:Consume[System.Numerics.Vector`1[int]](System.Numerics.Vector`1[int])
next:179     !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  error: match on wrong line

The LSRA change caused an extra mov v0.16b, v16.16b, but I have not been able to prioritize the destination register over op1 (any ideas would be appreciated). This was done to allow for movprfx usage, but in this case the ptrue suppressed it, so it doesn't really need to be delayed free. Maybe I should revert to using SVE mov instead of movprfx for now, and leave this for #107134.

Hey @ylpoonlg @a74nh following up on this. I may have lost some context, but during lsra we know whether the mask is all true, correct? In any case let me know if you needed further follow-up here, sorry for the delay

Thanks for looking at this. I think currently lsra is delay-freeing operands for all RMW instructions(#107134), but this is fine for now. The main issue here is the unary embedded masked operations, since they are not RMW but also support MOVPRFX. I attempted to enable its usage by allowing delay free in lsra, but looks like it is not assigning the registers optimally, causing the test failure:
For example with

abs z16.s, p0/m, z0.s
mov v0.16b, v16.16b

the extra mov is unnecessary if it can use the destination register directly and use a different source register:

abs z0.s, p0/m, z16.s

If this lsra is too complicated to fix in this PR, I can remove this particular optimization and not use MOVPRFX with unary embedded masked operations for now.

[a74nh]

@ylpoonlg : I've taken a look at this and made some fixes in LSRA, which fix up the regressions in both Abs and ConvertToSingleOdd. I don't have the permissions to push directly to this PR, so could you please take a look at my two commits and add them into this PR (assuming you're happy with the changes 😄 ).

https://github.com/dotnet/runtime/compare/main...a74nh:runtime:github-movprfx_refactor_3?expand=1

[dhartglassMSFT]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dhartglassMSFT]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dhartglassMSFT]

last regstress hit a known issue, kicking off a new one

[a74nh]

@dhartglassMSFT - did something go wrong with the stress run again?

[dhartglassMSFT]

/azp run runtime-coreclr jitstressregs

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dhartglassMSFT]

@dhartglassMSFT - did something go wrong with the stress run again?

It will surely work on the third try

[dhartglassMSFT]

@a74nh I took another look, unfortunately the SPMI failures appear related.

[01:51:17] ISSUE: <ASSERT> #61471 D:\a\_work\1\s\src\coreclr\jit\hwintrinsiccodegenarm64.cpp (824) - Assertion failed '(targetReg != intrin.op2->GetRegNum()) || genIsSameLocalVar(rmwOp, intrin.op2)' in 'SveBenchmarks.ComplexMultiply:SveComplexMultiply():this' during 'Generate code' (IL size 514; hash 0xcf12aa28; FullOpts)

[dhartglassMSFT]

@a74nh I had missed there were also these two in the SPMI output.

This now should be al the failures, apologies for missing these on my first glance yesterday
(same assert twice)

[17:22:24] ISSUE: <ASM_DIFF> main method 15075 of size 301 differs

[17:22:24] ISSUE: <ASM_DIFF> main method 18555 of size 535 differs

[17:22:24] ISSUE: <ASSERT> #21871 D:\a\_work\1\s\src\coreclr\jit\hwintrinsiccodegenarm64.cpp (854) - Assertion failed 'canRepairTargetOverlap || (targetReg != intrin.op2->GetRegNum()) || genIsSameLocalVar(rmwOp, intrin.op2)' in 'SveBenchmarks.ComplexMultiply:SveComplexMultiply():this' during 'Generate code' (IL size 514; hash 0xcf12aa28; FullOpts)

[17:22:24] 

[17:22:24] ISSUE: <ASM_DIFF> main method 11328 of size 148 differs

[17:22:24] ISSUE: <ASSERT> #17808 D:\a\_work\1\s\src\coreclr\jit\hwintrinsiccodegenarm64.cpp (854) - Assertion failed 'canRepairTargetOverlap || (targetReg != intrin.op2->GetRegNum()) || genIsSameLocalVar(rmwOp, intrin.op2)' in 'SveBenchmarks.Logarithm:SveLogarithm():this' during 'Generate code' (IL size 666; hash 0xfd45ed08; FullOpts)

[17:22:24] 

[17:22:24] Compilation failures

[a74nh]

@a74nh I had missed there were also these two in the SPMI output.

This now should be al the failures, apologies for missing these on my first glance yesterday (same assert twice)

[17:22:24] ISSUE: <ASM_DIFF> main method 15075 of size 301 differs

[17:22:24] ISSUE: <ASM_DIFF> main method 18555 of size 535 differs

[17:22:24] ISSUE: <ASSERT> #21871 D:\a\_work\1\s\src\coreclr\jit\hwintrinsiccodegenarm64.cpp (854) - Assertion failed 'canRepairTargetOverlap || (targetReg != intrin.op2->GetRegNum()) || genIsSameLocalVar(rmwOp, intrin.op2)' in 'SveBenchmarks.ComplexMultiply:SveComplexMultiply():this' during 'Generate code' (IL size 514; hash 0xcf12aa28; FullOpts)

[17:22:24] 

[17:22:24] ISSUE: <ASM_DIFF> main method 11328 of size 148 differs

[17:22:24] ISSUE: <ASSERT> #17808 D:\a\_work\1\s\src\coreclr\jit\hwintrinsiccodegenarm64.cpp (854) - Assertion failed 'canRepairTargetOverlap || (targetReg != intrin.op2->GetRegNum()) || genIsSameLocalVar(rmwOp, intrin.op2)' in 'SveBenchmarks.Logarithm:SveLogarithm():this' during 'Generate code' (IL size 666; hash 0xfd45ed08; FullOpts)

[17:22:24] 

[17:22:24] Compilation failures

These should be fixed now too...

[dhartglassMSFT]

I merged this, thanks @a74nh and @ylpoon-arm for the fix here!