[PATCH] D105473: [LV] Ignore candidate VFs with invalid costs.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6094-6096
     // Notice that the vector loop needs to be executed less times, so
     // we need to divide the cost of the vector loops by the width of
     // the vector elements.
----------------
dmgreen wrote:
> Is this comment still useful? It doesn't seem to relate to the code here.
Good spot, I've removed it.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6105-6106
+         << " because of instructions with invalid cost:\n";
+      for (const auto *I : InvalidCosts)
+        OS << "\t" << *I << "\n";
+      OS.flush();
----------------
dmgreen wrote:
> Do we usually add llvm instructions to optimization reports? Or do they usually use debug info for anything they print?
It is not common, but there is certainly precedent for it. For example:

llvm/test/CodeGen/AArch64/GlobalISel/arm64-fallback.ll:
  ; FALLBACK-WITH-REPORT-ERR: remark: <unknown>:0:0: unable to legalize instruction: G_STORE %3:_(<3 x s32>), %4:_(p0) :: (store 12 into %ir.addr + 16, align 16, basealign 32) (in function: odd_vector)                                                                                                              

llvm/test/CodeGen/X86/fast-isel-abort-warm.ll:
  ; CHECK: remark: <unknown>:0:0: FastISel missed call:   call void asm sideeffect


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8040-8041
+    if (!CM.hasInvalidCosts(UserVF)) {
+      LLVM_DEBUG(dbgs() << "LV: Using "
+                        << "user VF " << UserVF << ".\n");
+      CM.collectInLoopReductions();
----------------
dmgreen wrote:
> Reflow comment.
> 
> Will this work for reductions if it's placed here? Can an inloop reduction have an invalid cost? Or are they OK because there is a target hook for them?
It would, although at the moment the code in getInstructionCost seems to ignore the `Invalid` returned by `getReductionPatternCost`, so that will need a bit of work. It's fine though, since the `canVectorizeReductions` function (which calls a TTI hook) covers this case already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105473