[PATCH] D101916: [LoopVectorize] Fix crash for predicated instructions with scalable VF

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Thanks for the changes! Just a few little nits left.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6831
+        if (!VF.isScalable() && !useEmulatedMaskMemRefHack(&I))
+          if (computePredInstDiscount(&I, ScalarCosts, VF) >= 0)
+            ScalarCostsVF.insert(ScalarCosts.begin(), ScalarCosts.end());
----------------
nit: this can just be part of the same expression, i.e.

  if (!VF.isScalable() && !useEmulatedMaskMemRefHack(&I) &&
      computePredInstDiscount(&I, ScalarCosts, VF) >= 0)


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6908
     InstructionCost ScalarCost =
-        VF.getKnownMinValue() *
+        VF.getFixedValue() *
         getInstructionCost(I, ElementCount::getFixed(1)).first;
----------------
Can you split out all the changes to use `getFixedValue()` from this patch and commit those as a separate NFC patch?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-predicate-instruction.ll:1
+; RUN: opt < %s -loop-vectorize -scalable-vectorization=on -mattr=+sve -mtriple aarch64-unknown-linux-gnu -S | FileCheck %s
+; RUN: opt < %s -loop-vectorize -scalable-vectorization=on -mattr=+sve -mtriple aarch64-unknown-linux-gnu -prefer-predicate-over-epilogue=predicate-dont-vectorize -S | FileCheck %s
----------------
nit: Can you remove the `-mattr=+sve -mtriple aarch64-unknown-linux-gnu` and instead encode it in the IR as:

  target triple = aarch64-unknown-linux-gnu
  
  define void @predication_in_loop(...) #0 {
    ..
  }
  
  attributes #0 = { "target-features"="+sve" }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101916