[PATCH] D99569: [LoopVectorize] Fix bug where predicated loads/stores were dropped

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalarize-store-with-predication.ll:3
+; RUN: opt -loop-vectorize -dce -instcombine \
+; RUN:     -debug-only=loop-vectorize \
+; RUN:     -S -o - 2>%t < %s | FileCheck %s
----------------
peterwaller-arm wrote:
> I suspect using `-debug` needs a `REQUIRES: asserts` line, since the tests will be run presumably with a release build where -debug is unavailable.
> 
> The output of:
> 
> rg -C3  -- 'RUN.* -debug-only'
> 
> ... in the llvm/test directory appears to show that they generally have such a REQUIRES line.
> 
> I find this a little surprising since presumably this doesn't work on an asserts+release build; but my reading is that enabling assertions may enable debug output: 
> 
> https://github.com/llvm/llvm-project/blob/8396aeb07cddd8ab9a6a154a4ab7ac56fc24bda5/llvm/cmake/modules/HandleLLVMOptions.cmake#L61-L76
@peterwaller-arm yep you're right this needs:

  REQUIRES: asserts

and it will enable debug.


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/x86-predication.ll:69
+; SINK-GATHER: pred.load.if:
+; SINK-GATHER:   %{{.*}} = load i32, i32* %{{.*}}, align 4
+; SINK-GATHER: pred.load.continue:
----------------
Hi @joechrisellis you've lost the variable `T0` here that was used to ensure the load fed into the udiv. It would be good if you could re-add this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99569