[PATCH] D12259: Improved Diagnostics and Extended vectorize(enable)

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a subscriber: mzolotukhin.
mzolotukhin added a comment.

Hi Tyler,

Please find some comments/questions from me inline:


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:32-33
@@ +31,4 @@
+    "force-vectorization", cl::Hidden,
+    cl::desc("Override vectorizer cost-model and/or allow floating-point and "
+             "memory operations to be reordered. Negative one is autoselect."),
+    cl::location(VectorizerParams::VectorizationForced), cl::init(-1));
----------------
>From the description it's not clear what the number stands for (I'd expect `force-vectorization` to be boolean). Could you please reword this somehow?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:847
@@ -841,2 +846,3 @@
         Interleave("interleave.count", DisableInterleaving, HK_UNROLL),
-        Force("vectorize.enable", FK_Undefined, HK_FORCE),
+        Force("vectorize.enable", VectorizerParams::VectorizationForced,
+              HK_FORCE),
----------------
Why do we change this (default?) value?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:933-934
@@ +932,4 @@
+      return LV_NAME;
+    if (getForce() == LoopVectorizeHints::FK_Undefined && getWidth() == 0)
+      return LV_NAME;
+    return DiagnosticInfo::AlwaysPrint;
----------------
Don't we start returning `AlwaysPrint` for all vectorized loops if no hint is specified (`getForce() == LoopVectorizeHints::FK_Undefined`, `getWidth() == 4/8/16`)?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1538
@@ +1537,3 @@
+                                VectorizerParams::RuntimeMemoryCheckThreshold &&
+                            !Hints.allowReordering();
+    if (ThresholdReached || PragmaThresholdReached) {
----------------
Why is `allowReordering` a part of `ThresholdReached`?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1823-1825
@@ -1803,5 +1822,5 @@
       // Do not vectorize or interleaving the loop.
-      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+      emitOptimizationRemarkAnalysis(F->getContext(), VAPassName, *F,
                                      L->getStartLoc(), VecDiagMsg);
-      emitOptimizationRemarkAnalysis(F->getContext(), DEBUG_TYPE, *F,
+      emitOptimizationRemarkAnalysis(F->getContext(), LV_NAME, *F,
                                      L->getStartLoc(), IntDiagMsg);
----------------
Why do we distinguish these two? If we don't want to see one of them in -R-reports, should we just make it `DEBUG(dbgs() << "...")` instead of `emitOptimizationRemarkAnalysis`?


http://reviews.llvm.org/D12259