[PATCH] D126295: [LoopVectorize] Fix assertion failure in fixReduction when tail-folding

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.

LGTM, thanks! It would probably be more descriptive to say how the issue is fixed in the commit message, e.g. `Skip stores of reduction values before trying to widen` or something like that.



================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-reductions-tf.ll:2
+; RUN: opt < %s -loop-vectorize -pass-remarks=loop-vectorize -pass-remarks-analysis=loop-vectorize -pass-remarks-missed=loop-vectorize \
+; RUN:   -prefer-predicate-over-epilogue=predicate-dont-vectorize -mtriple aarch64-unknown-linux-gnu -mattr=+sve,+bf16 -S 2>%t | FileCheck %s -check-prefix=CHECK
+; RUN: cat %t | FileCheck %s -check-prefix=CHECK-REMARK
----------------
no need to add `+bf16`?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-reductions-tf.ll:3
+; RUN:   -prefer-predicate-over-epilogue=predicate-dont-vectorize -mtriple aarch64-unknown-linux-gnu -mattr=+sve,+bf16 -S 2>%t | FileCheck %s -check-prefix=CHECK
+; RUN: cat %t | FileCheck %s -check-prefix=CHECK-REMARK
+
----------------
is there any particular reason you are checking for remarks?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-reductions-tf.ll:29
+  %tobool7.not = icmp eq i32 %load, 0
+  br i1 %tobool7.not, label %if.else, label %if.then8
+
----------------
the control flow in the loop shouldn't be needed.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-reductions-tf.ll:49
+
+for.end:                                          ; preds = %for.end.loopexit, %if.then4
+  ret void
----------------
`%if.then4` comment here seems out of date, `if.then4` doesn't exist? Same for `for.body.lr.ph` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126295