[SVE][codegen] Add few more tests for MUL followed by ADD/SUB (NFC)

[sushgokh]

Summary

These tests will form the base for upcoming patch for generating pseudo instructions for MLA/MAD/MLS/MSB at ISel.

This is also forms base for attempt made in D142656.

[SjoerdMeijer]

llvm/test/CodeGen/AArch64/sve-multiply-add-accumulate.ll

12

if I am not mistaken, the other patch will change this to

; CHECK-LABEL: muladd_i64:
; CHECK:       // %bb.0:
; CHECK-NEXT:    ptrue p0.d
; CHECK-NEXT:    mov z2.d, #1 // =0x1
; CHECK-NEXT:    mad z0.d, p0/m, z1.d, z2.d
; CHECK-NEXT:    ret

(perhaps the immediate is different but that looks irrelevant to me).

This new codegen looks better, it has 1 mov less, but that is just to return the value. There is no real use of the value, which makes this move visible. Basically what I am saying is that it is unclear what the benefit is.

But either way, I will repeat my request on the other patch: let's separate things and let this be about instruction selection of 'mad'. This means that we only want to do have 'mad' tests here, and separate out the mla -> mad changes.

[sushgokh]

llvm/test/CodeGen/AArch64/sve-multiply-add-accumulate.ll

12

I can just commit the tests for mad.

But the changes for D142656 have effect on tests that are currently generating mla. If I commit tests for mla+mad (as I have done here), comparison will be easier for D142656

[SjoerdMeijer]

I think we need at least 3 patches:

1. Instruction selection of mad for pattern add ( mul, splat_vector(C)). That is D142656.
2. New tests for this mad pattern and isel. That is this patch, D142998.

So the above only deals with mad, and not with any mla -> mad changes. That's why I suggested to strip out any mla changes out of patches 1 and 2. If you want to make changes in this area, we will follow the same approach:

3. Create a patch to precommit new test, if applicable.
4. Create a patch that implements these mla -> mad changes.

What do you think, makes sense?

[sushgokh]

In D142998#4095821, @SjoerdMeijer wrote:

I think we need at least 3 patches:

1. Instruction selection of mad for pattern add ( mul, splat_vector(C)). That is D142656.
2. New tests for this mad pattern and isel. That is this patch, D142998.

So the above only deals with mad, and not with any mla -> mad changes. That's why I suggested to strip out any mla changes out of patches 1 and 2. If you want to make changes in this area, we will follow the same approach:

3. Create a patch to precommit new test, if applicable.
4. Create a patch that implements these mla -> mad changes.

What do you think, makes sense?

I tried seperating (1,2) and (3,4) as you say above. However, mla->mad is side effect of implementing (1) and this makes seperating (1) and (4) a difficult thing unless we introduce some hacks to do so

[SjoerdMeijer]

In D142998#4095916, @sushgokh wrote:

I tried seperating (1,2) and (3,4) as you say above. However, mla->mad is side effect of implementing (1) and this makes seperating (1) and (4) a difficult thing unless we introduce some hacks to do so

Ok, I will need to understand better why we can't separate things. But I will go back to the other patch for that.

[david-arm]

Thanks for putting these tests in a precommit patch @sushgokh!

llvm/test/CodeGen/AArch64/sve-multiply-add-accumulate.ll

1

Perhaps it's better to just add these tests to the existing sve-int-mad-pred.ll file?

165

I'm not sure what this is doing that is different to @muladd_i8_negativeAddend? Are you just trying to test a case where the immediate is too big to fit into the 'add'?

[sushgokh]

llvm/test/CodeGen/AArch64/sve-multiply-add-accumulate.ll

1

Just thought better to have seperate file because sve-int-mad-pred.ll only has intrinsics. If you suggest, will add all the tests to that file

165

In this test, unlike the other tests, immediate is realised using 2 register moves.

[sushgokh]

Adding a loop based test case to understand the effect of patch D142656 better

[sushgokh]

@paulwalker-arm adding test cases for

1. Generating pseudo inst for MLA/MAD/MLS/MSUB
2. Basic test cases on mul + add/sub as you had suggested in other patch

[sushgokh]

@paulwalker-arm Any comments/suggestions on this ?

[paulwalker-arm]

In D142998#4170452, @sushgokh wrote:

@paulwalker-arm Any comments/suggestions on this ?

In truth I've ignored this patch whilst waiting on your other patch. My review comments on D142656 include the test file, which I'm recommending to follow the same structure as was used for the FMAs. Those being simple tests I'm happy for them to remain included with the patch that improves the isel. I think this patch can then be rebased on top of D142656 to clearly show the extra cases you care about.

[sushgokh]

In D142998#4170949, @paulwalker-arm wrote:

In D142998#4170452, @sushgokh wrote:

@paulwalker-arm Any comments/suggestions on this ?

In truth I've ignored this patch whilst waiting on your other patch. My review comments on D142656 include the test file, which I'm recommending to follow the same structure as was used for the FMAs. Those being simple tests I'm happy for them to remain included with the patch that improves the isel. I think this patch can then be rebased on top of D142656 to clearly show the extra cases you care about.

@paulwalker-arm As agreed in last comment for D142656,

1. Motive of this patch is converting to pseudo instructions. I was assuming these are fairly simple tests with one of opernds as shuffle. If you just expect one liners, I think tests are already present for this patch. Agreed ?

So, to reiterate the line of action,

1. This patch will commit tests for converting to pseudo instructions
2. Next patch will actually convert to pseudo instructions
3. Patch for converting to FMA in D142656. I am not addressing D142656 immediately

[paulwalker-arm]

Fair enough. Sorry for the misunderstanding.

llvm/test/CodeGen/AArch64/sve-generate-pseudo.ll

4 ↗

(On Diff #499778)

Please use update_llc_test_checks.py to autogenerate the CHECK lines. Or perhaps why not just place all the tests within sve-int-arith.ll? given they're related.

10 ↗

(On Diff #499778)

Please include equivalent tests for the other supports element types (i.e. i16, i32 and i64).

[paulwalker-arm]

llvm/test/CodeGen/AArch64/sve-generate-pseudo.ll

10 ↗

(On Diff #499778)

I forgot to mention. Please also add equivalent tests that are expected to result in FMAD/FMSB being emitted once the follow on patch lands.

[sushgokh]

llvm/test/CodeGen/AArch64/sve-generate-pseudo.ll

4 ↗

(On Diff #499778)

I am not autogenerating because we just want to check the pseudo instruction thats generated. I suppose we arent interested in other instructions. Will add tests for other types.

I have not added them to sve-int-arith.ll for reason that:

1. Want to avoid auto-generation as said above
2. File name clearly states the purpose

10 ↗

(On Diff #499778)

agreed. Thanks

[sushgokh]

llvm/test/CodeGen/AArch64/sve-generate-pseudo.ll

10 ↗

(On Diff #499778)

Current tests in sve-generate-pseudo.ll are for MAD/MSB once the follow on patch lands. i.e. currently, they are generating MLA/MLS but once the follow on patch lands, they will result in MAD/MSB being generated.

So, thanks. Will add assembly instructions as well in the check lines to indicate actual instruction generated

[paulwalker-arm]

llvm/test/CodeGen/AArch64/sve-generate-pseudo.ll

4 ↗

(On Diff #499778)

The rational here does not make sense to me as the use of pseudo instructions it not revenant from a testing point of view. These tests are simply to show the expected output from a given blob of IR.

That said, after looking at sve-int-arith.ll I can see now that these are just clones of existing tests (mla_i8 and mls_i8) and so what I'm really asking for is this patch to extend those tests within sve-int-arith.ll to cover the other element types for all of mla, mad, mls, and msb.

[paulwalker-arm]

llvm/test/CodeGen/AArch64/sve-int-arith.ll

402–403 ↗

(On Diff #502734)

Does simplify the mla tests to:

%prod = mul <vscale x 2 x i64> %b, %c
%res = add <vscale x 2 x i64> %a, %prod
ret <vscale x 2 x i64> %res

give the desired output?

[paulwalker-arm]

A few recommended improvements but otherwise looks good.

llvm/test/CodeGen/AArch64/sve-int-arith.ll

342 ↗

(On Diff #502734)

Please name the tests after the expected resulting instruction, so mad in this case.

526–530 ↗

(On Diff #502734)

As above, I think this can be just:

%prod = mul <vscale x 8 x i16> %b, %c
%res = sub <vscale x 8 x i16> %a, %prod
ret <vscale x 8 x i16> %res

[sushgokh]

llvm/test/CodeGen/AArch64/sve-int-arith.ll

342 ↗

(On Diff #502734)

So, I have named it as per the current instruction. Once the pseudo instr generation patch lands, will name as per appropriate instruction. Sounds good?

402–403 ↗

(On Diff #502734)

Yes, it works. Thanks. WIll update tests.

llvm/test/CodeGen/AArch64/sve-multiply-add-accumulate.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64-unknown-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+define <vscale x 2 x i64> @muladd_i64_positiveAddend(<vscale x 2 x i64> %a, <vscale x 2 x i64> %b)
+; CHECK-LABEL: muladd_i64_positiveAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.d
+; CHECK-NEXT:    mov z2.d, #0xffffffff
+; CHECK-NEXT:    mla z2.d, p0/m, z0.d, z1.d
+; CHECK-NEXT:    mov z0.d, z2.d
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 2 x i64> %a, %b
+  %2 = add  <vscale x 2 x i64> %1, shufflevector (<vscale x 2 x i64> insertelement (<vscale x 2 x i64> poison, i64 4294967295, i64 0), <vscale x 2 x i64> poison, <vscale x 2 x i32> zeroinitializer)
+  ret <vscale x 2 x i64> %2
+}
+
+define <vscale x 2 x i64> @muladd_i64_negativeAddend(<vscale x 2 x i64> %a, <vscale x 2 x i64> %b)
+; CHECK-LABEL: muladd_i64_negativeAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.d
+; CHECK-NEXT:    mov z2.d, #0xffffffff00000001
+; CHECK-NEXT:    mla z2.d, p0/m, z0.d, z1.d
+; CHECK-NEXT:    mov z0.d, z2.d
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 2 x i64> %a, %b
+  %2 = add  <vscale x 2 x i64> %1, shufflevector (<vscale x 2 x i64> insertelement (<vscale x 2 x i64> poison, i64 -4294967295, i64 0), <vscale x 2 x i64> poison, <vscale x 2 x i32> zeroinitializer)
+  ret <vscale x 2 x i64> %2
+}
+
+define <vscale x 4 x i32> @muladd_i32_positiveAddend(<vscale x 4 x i32> %a, <vscale x 4 x i32> %b)
+; CHECK-LABEL: muladd_i32_positiveAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.s
+; CHECK-NEXT:    mov z2.s, #0x10000
+; CHECK-NEXT:    mla z2.s, p0/m, z0.s, z1.s
+; CHECK-NEXT:    mov z0.d, z2.d
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 4 x i32> %a, %b
+  %2 = add  <vscale x 4 x i32> %1, shufflevector (<vscale x 4 x i32> insertelement (<vscale x 4 x i32> poison, i32 65536, i32 0), <vscale x 4 x i32> poison, <vscale x 4 x i32> zeroinitializer)
+  ret <vscale x 4 x i32> %2
+}
+
+define <vscale x 4 x i32> @muladd_i32_negativeAddend(<vscale x 4 x i32> %a, <vscale x 4 x i32> %b)
+; CHECK-LABEL: muladd_i32_negativeAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.s
+; CHECK-NEXT:    mov z2.s, #0xffff0000
+; CHECK-NEXT:    mla z2.s, p0/m, z0.s, z1.s
+; CHECK-NEXT:    mov z0.d, z2.d
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 4 x i32> %a, %b
+  %2 = add  <vscale x 4 x i32> %1, shufflevector (<vscale x 4 x i32> insertelement (<vscale x 4 x i32> poison, i32 -65536, i32 0), <vscale x 4 x i32> poison, <vscale x 4 x i32> zeroinitializer)
+  ret <vscale x 4 x i32> %2
+}
+
+define <vscale x 8 x i16> @muladd_i16_positiveAddend(<vscale x 8 x i16> %a, <vscale x 8 x i16> %b)
+; CHECK-LABEL: muladd_i16_positiveAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    mul z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT:    add z0.h, z0.h, #255 // =0xff
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 8 x i16> %a, %b
+  %2 = add  <vscale x 8 x i16> %1, shufflevector (<vscale x 8 x i16> insertelement (<vscale x 8 x i16> poison, i16 255, i16 0), <vscale x 8 x i16> poison, <vscale x 8 x i32> zeroinitializer)
+  ret <vscale x 8 x i16> %2
+}
+
+define <vscale x 8 x i16> @muladd_i16_negativeAddend(<vscale x 8 x i16> %a, <vscale x 8 x i16> %b)
+; CHECK-LABEL: muladd_i16_negativeAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    mov z2.h, #-255 // =0xffffffffffffff01
+; CHECK-NEXT:    mla z2.h, p0/m, z0.h, z1.h
+; CHECK-NEXT:    mov z0.d, z2.d
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 8 x i16> %a, %b
+  %2 = add  <vscale x 8 x i16> %1, shufflevector (<vscale x 8 x i16> insertelement (<vscale x 8 x i16> poison, i16 -255, i16 0), <vscale x 8 x i16> poison, <vscale x 8 x i32> zeroinitializer)
+  ret <vscale x 8 x i16> %2
+}
+
+define <vscale x 16 x i8> @muladd_i8_positiveAddend(<vscale x 16 x i8> %a, <vscale x 16 x i8> %b)
+; CHECK-LABEL: muladd_i8_positiveAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.b
+; CHECK-NEXT:    mul z0.b, p0/m, z0.b, z1.b
+; CHECK-NEXT:    add z0.b, z0.b, #15 // =0xf
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 16 x i8> %a, %b
+  %2 = add  <vscale x 16 x i8> %1, shufflevector (<vscale x 16 x i8> insertelement (<vscale x 16 x i8> poison, i8 15, i8 0), <vscale x 16 x i8> poison, <vscale x 16 x i32> zeroinitializer)
+  ret <vscale x 16 x i8> %2
+}
+
+define <vscale x 16 x i8> @muladd_i8_negativeAddend(<vscale x 16 x i8> %a, <vscale x 16 x i8> %b)
+; CHECK-LABEL: muladd_i8_negativeAddend:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.b
+; CHECK-NEXT:    mul z0.b, p0/m, z0.b, z1.b
+; CHECK-NEXT:    add z0.b, z0.b, #241 // =0xf1
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 16 x i8> %a, %b
+  %2 = add  <vscale x 16 x i8> %1, shufflevector (<vscale x 16 x i8> insertelement (<vscale x 16 x i8> poison, i8 -15, i8 0), <vscale x 16 x i8> poison, <vscale x 16 x i32> zeroinitializer)
+  ret <vscale x 16 x i8> %2
+}
+
+; both mul operands have a use
+define <vscale x 8 x i16> @muladd_generic_test1(<vscale x 8 x i16> %a, <vscale x 8 x i16> %b)
+; CHECK-LABEL: muladd_generic_test1:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    movprfx z2, z0
+; CHECK-NEXT:    mul z2.h, p0/m, z2.h, z1.h
+; CHECK-NEXT:    add z2.h, z2.h, #200 // =0xc8
+; CHECK-NEXT:    mul z0.h, p0/m, z0.h, z2.h
+; CHECK-NEXT:    sub z0.h, z0.h, z1.h
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 8 x i16> %a, %b
+  %2 = add  <vscale x 8 x i16> %1, shufflevector (<vscale x 8 x i16> insertelement (<vscale x 8 x i16> poison, i16 200, i16 0), <vscale x 8 x i16> poison, <vscale x 8 x i32> zeroinitializer)
+  %3 = mul <vscale x 8 x i16> %2, %a
+  %4 = sub <vscale x 8 x i16> %3, %b
+  ret <vscale x 8 x i16> %4
+}
+
+; only the first mul operand has a use
+define <vscale x 8 x i16> @muladd_generic_test2(<vscale x 8 x i16> %a, <vscale x 8 x i16> %b)
+; CHECK-LABEL: muladd_generic_test2:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    mul z1.h, p0/m, z1.h, z0.h
+; CHECK-NEXT:    add z1.h, z1.h, #200 // =0xc8
+; CHECK-NEXT:    mul z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 8 x i16> %a, %b
+  %2 = add  <vscale x 8 x i16> %1, shufflevector (<vscale x 8 x i16> insertelement (<vscale x 8 x i16> poison, i16 200, i16 0), <vscale x 8 x i16> poison, <vscale x 8 x i32> zeroinitializer)
+  %3 = mul <vscale x 8 x i16> %2, %a
+  ret <vscale x 8 x i16> %3
+}
+
+; only the second mul operand has a use
+define <vscale x 8 x i16> @muladd_generic_test3(<vscale x 8 x i16> %a, <vscale x 8 x i16> %b)
+; CHECK-LABEL: muladd_generic_test3:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    mul z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT:    add z0.h, z0.h, #200 // =0xc8
+; CHECK-NEXT:    mul z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 8 x i16> %a, %b
+  %2 = add  <vscale x 8 x i16> %1, shufflevector (<vscale x 8 x i16> insertelement (<vscale x 8 x i16> poison, i16 200, i16 0), <vscale x 8 x i16> poison, <vscale x 8 x i32> zeroinitializer)
+  %3 = mul <vscale x 8 x i16> %2, %b
+  ret <vscale x 8 x i16> %3
+}
+
+; negative integer splat as one of the addend
+define <vscale x 8 x i16> @muladd_generic_test4(<vscale x 8 x i16> %a, <vscale x 8 x i16> %b)
+; CHECK-LABEL: muladd_generic_test4:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #-200
+; CHECK-NEXT:    ptrue p0.h
+; CHECK-NEXT:    mov z2.h, w8
+; CHECK-NEXT:    mla z2.h, p0/m, z0.h, z1.h
+; CHECK-NEXT:    mov z0.d, z2.d
+; CHECK-NEXT:    ret
+{
+  %1 = mul <vscale x 8 x i16> %a, %b
+  %2 = add  <vscale x 8 x i16> %1, shufflevector (<vscale x 8 x i16> insertelement (<vscale x 8 x i16> poison, i16 -200, i16 0), <vscale x 8 x i16> poison, <vscale x 8 x i32> zeroinitializer)
+  ret <vscale x 8 x i16> %2
+}