[PATCH] D38367: [SLP] Added more missed optimiazation remarks

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks for working on this! I left a few comments.



================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4459
+      return false;
+    if (Inst->getOpcode() != Opcode0) {
+      // FIXME: need more user-friendly message here
----------------
The code in the if branch does not use Inst, so could we keep `    if (!Inst || Inst->getOpcode() != Opcode0)` ?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4460
+    if (Inst->getOpcode() != Opcode0) {
+      // FIXME: need more user-friendly message here
+      R.getORE()->emit(OptimizationRemarkMissed(SV_NAME, "InequableTypes", I0)
----------------
What would a better error message look like? We emit it if the the opcodes do not match. Would it make sense to add the opcodes to the message?


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4575
+    R.getORE()->emit(OptimizationRemarkMissed(SV_NAME, "NotPossible", I0)
+                     << "Cannot vectorize list: vectorization was impossible"
+                     << " with available vectorization factors");
----------------
I think we should be consistent with the messages. At the beginning you use something  like `Cannot SLP vectorize list`


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:5272
+
+      auto *I0 = cast<Instruction>(VL[0]);
+
----------------
This is only used by the remark as far as I can see. I think it would be simpler to just to the cast when constructing the remark.


================
Comment at: test/Transforms/SLPVectorizer/X86/remark_horcost.ll:1
+; RUN: opt -S -mtriple=x86_64-pc-linux-gnu -mcpu=generic -slp-vectorizer -pass-remarks-missed=slp-vectorizer -o /dev/null < %s 2>&1 | FileCheck %s
+
----------------
Please consider using the YAML format when in the test, like in rL302811


Repository:
  rL LLVM

https://reviews.llvm.org/D38367