[PATCH] D62997: [LV] Share the LV illegality reporting with LoopVectorize. NFC.

[Hideki Saito via Phabricator via llvm-commits]

hsaito added a comment.

Sorry, I missed the original review request in the e-mail pile up. Yes, this is the direction I was suggesting. Thank you.

My preference is to have the utility declared in LoopVectorize.h as a more neutral place. If we had vectorizer utility .h/.cpp, that would have been ideal, What bothers me is Hints.vectorizeAnalysisPassName(). Anybody know why we use AlwaysPrint()?

Not a big fan of creating a new entry just for two parameters --- especially when those two parameters are probably the same for all cases. If we can always use LV_NAME, one of the two parameters can be eliminated.
If theLoop is the only thing left, passing it from all calls seems better than having two functions with the same name with same functionality (on in class and one at top level).



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7339
   if (F->hasFnAttribute(Attribute::NoImplicitFloat)) {
-    LLVM_DEBUG(dbgs() << "LV: Can't vectorize when the NoImplicitFloat"
-                         "attribute is used.\n");
-    ORE->emit(createLVMissedAnalysis(Hints.vectorizeAnalysisPassName(),
-                                     "NoImplicitFloat", L)
-              << "loop not vectorized due to NoImplicitFloat attribute");
+    ::reportVectorizationFailure(
+        "Can't vectorize when the NoImplicitFloat attribute is used",
----------------
Do we need :: here?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7353
       TTI->isFPVectorizationPotentiallyUnsafe()) {
-    LLVM_DEBUG(
-        dbgs() << "LV: Potentially unsafe FP op prevents vectorization.\n");
-    ORE->emit(
-        createLVMissedAnalysis(Hints.vectorizeAnalysisPassName(), "UnsafeFP", L)
-        << "loop not vectorized due to unsafe FP support.");
+    ::reportVectorizationFailure(
+        "Potentially unsafe FP op prevents vectorization",
----------------
Same as above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62997