[PATCH] D105824: [LV] Avoid scalable vectorization for loops containing alloca

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7898
+    // the result would need to be a vector of pointers.
+    // Return an Invalid cost if the VF is Scalable.
+    if (VF.isScalable())
----------------
nit: this line is redundant, because the code says as much.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll:3
+; RUN: FileCheck %s --check-prefix=CHECK-REMARKS < %t
+
+; CHECK-REMARKS: Instruction with invalid costs prevented vectorization at VF=(vscale x 1, vscale x 2): alloca
----------------
Can you also add: `CHECK-REMARKS: UserVF ignored because of invalid costs.`


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll:8
+; CHECK-LABEL: @alloca(
+; CHECK:       vector.body:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, %vector.ph ], [ [[INDEX_NEXT:%.*]], %vector.body ]
----------------
I'd suggest to remove these CHECK lines, because the fact that this loop vectorizes with fixed-width vectors is down to the cost-model, which means we may have to regenerate the check lines at some point, even though they're not needed for what it is that you're trying to test.


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll:1
+; RUN: opt -S -loop-vectorize -mattr=+sve -mtriple aarch64-unknown-linux-gnu -scalable-vectorization=on < %s | FileCheck %s
+
----------------
CarolineConcatto wrote:
> Makes sense to use the python file to create the check lines here?
@CarolineConcatto I don't think that is necessary for this specific patch because this test is not guarding the fixed-width vectorization capabilities of the LV, nor the AArch64 CostModel.


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

https://reviews.llvm.org/D105824