[PATCH] D101836: [LoopVectorize] Enable strict reductions when allowReordering() returns false

[Kerry McLaughlin via Phabricator via llvm-commits]

kmclaughlin added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll:74
+; CHECK-VF8: %[[STEPVEC1:.*]] = call <vscale x 8 x i64> @llvm.experimental.stepvector.nxv8i64()
+; CHECK-VF8: %[[STEPVEC_ADD1:.*]] = add <vscale x 8 x i64> %[[STEPVEC1]], shufflevector (<vscale x 8 x i64> insertelement (<vscale x 8 x i64> poison, i64 0, i32 0), <vscale x 8 x i64> poison, <vscale x 8 x i32> zeroinitializer)
+; CHECK-VF8: %[[STEPVEC_MUL:.*]] = mul <vscale x 8 x i64> %[[STEPVEC_ADD1]], shufflevector (<vscale x 8 x i64> insertelement (<vscale x 8 x i64> poison, i64 2, i32 0), <vscale x 8 x i64> poison, <vscale x 8 x i32> zeroinitializer)
----------------
sdesmalen wrote:
> Why does this test need to be vectorized with VF=vscale x 8 instead of VF=vscale x 4? Is that because it needs to be driven using the cmdline flags to circumvent the "hint allows reordering" behaviour? (and so that the 1 RUN line covers all tests?) If that's the case, can you do a NFC patch where you first change the test to use the new VF, and then rebase this patch on top?
Hi @sdesmalen, it is the case that I changed the VF so that the test could be covered by one RUN line, and to try and circumvent the hints allow reordering behaviour. I will move changes to the RUN lines into a new patch so that this patch only adds new tests.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/strict-fadd.ll:1
-; RUN: opt < %s -loop-vectorize -mtriple aarch64-unknown-linux-gnu -enable-strict-reductions -S | FileCheck %s -check-prefix=CHECK
+; RUN: opt < %s -loop-vectorize -mtriple aarch64-unknown-linux-gnu -enable-strict-reductions -force-vector-width=8 -force-vector-interleave=1 -S | FileCheck %s -check-prefix=CHECK-VF8UF1
+; RUN: opt < %s -loop-vectorize -mtriple aarch64-unknown-linux-gnu -enable-strict-reductions -force-vector-width=8 -force-vector-interleave=4 -S | FileCheck %s -check-prefix=CHECK-VF8UF4
----------------
sdesmalen wrote:
> The first test, `@fadd_strict`, has check lines that match the first RUN line, and the 4th RUN line, but none of the others. Why is that?
> And are all these RUN lines needed?
Since the allowReordering() function returns false if `EC.getKnownMinValue() > 1`, I thought it was worth making sure that we don't vectorize a VF of 1 for at least one of the tests, which is why I added the extra RUN line to `@fadd_strict`.
The RUN lines are needed so that we can pass the different VFs & interleave counts needed for each of the tests (e.g. `@fadd_strict_unroll` needs a UF > 1) and I didn't want to change the 'allow reordering' behaviour by passing hints through metadata. Though I think I could remove the `CHECK-PRED` line since the `@fadd_predicated` does rely on metadata if this would help at all?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101836/new/

https://reviews.llvm.org/D101836