[PATCH] D103015: [NFC] Add CHECK lines for unordered FP reductions

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

I think the patch looks good to me, but I'll hold off a LGTM for now since I justed realise you've got other comments too.



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll:131
+; CHECK-UNORDERED: %[[LOAD2:.*]] = load float, float* %[[ARRAYIDX]]
+; CHECK-UNORDERED: vector.ph
+; CHECK-UNORDERED: %[[STEPVEC1:.*]] = call <vscale x 4 x i64> @llvm.experimental.stepvector.nxv4i64()
----------------
nit: It looks like none of the variables in the `vector.ph` is actually used further down. Maybe it's possible to just delete the whole `vector.ph` block since it's tested in ORDERED case anyway, unless you're now going to add PHIs to the vector.body which show INDUCTION being used?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll:187
 
 define float @fadd_invariant(float* noalias nocapture readonly %a, float* noalias nocapture readonly %b, i64 %n) {
+; CHECK-ORDERED-LABEL: @fadd_invariant
----------------
nit: I realise you've not changed the test name here, but I think maybe the name is a little confusing because there isn't actually anything invariant in the loop itself. Although I do realise this probably came from C code where an invariant condition was being tested in the loop and got hoisted out. I think what's actually being tested here is that we can vectorise loops with normal fadds feeding into a reducing fadd, i.e.

  for (i = 0; i < n; i++) {
    r += a[i] + b[i];
  }

Maybe it's worth changing the name as part of this patch to something like `fadd_of_fadds` since it's still NFC?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/strict-fadd.ll:262
 
 define float @fadd_invariant(float* noalias nocapture readonly %a, float* noalias nocapture readonly %b, i64 %n) {
+; CHECK-ORDERED-LABEL: @fadd_invariant
----------------
nit: Same comment as in the scalable test. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103015