[PATCH] [LoopVectorize]Teach Loop Vectorizer about interleaved memory access

hfinkel at anl.gov hfinkel at anl.gov
Tue May 5 18:57:51 PDT 2015


================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:313
@@ +312,3 @@
+    // lanes.
+    Cost += VecType->getNumElements();
+    return Cost;
----------------
This is not really the right place for this default, and not the right way to generate it. Just return 1 here.

The default should be in include/llvm/CodeGen/BasicTTIImpl.h, and you should call getScalarizationOverhead, likely, to generate the default cost.

================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:533
@@ -532,2 +532,3 @@
           *Ptr << " SCEV: " << *PtrScev << "\n");
+    return 0;
   }
----------------
Is this just a drive-by bug fix?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:164
@@ +163,3 @@
+static cl::opt<bool>
+    VectorizeInterleavedAccess("vectorize-interleaved-access", cl::init(true),
+                               cl::Hidden,
----------------
rengolin wrote:
> If we need a flag for the moment, it's to make it false by default, so we can turn it on when we want, not the other way around. Later, if the pass has proven correct, we should make it on by default. This transformation won't change unit stride loops anyway, so I don't see why we would need to disable it even temporarily.
> 
> Another note, I don't think we need the extensive comment here, just one line line the others will be fine. Nor we should document *how* we vectorize, since if that changes, the comment will be automatically obsolete, and probably never changed.
I disagree, the comment is good and we definitely should have it. However, it should not be here, but with the implementation (where it is also much less likely to get out of sync).

Luckily, it seems that we already have such comments below, so this might be just removed. However, I don't want us to give the impression that extensive explanatory comments are undesirable in general.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3934
@@ +3933,3 @@
+
+  for (SetVector<Instruction *>::iterator it = Heads.begin(), e = Heads.end();
+       it != e; ++it) {
----------------
You can use auto here for the iterator type, a range-based for would be better.

http://reviews.llvm.org/D9368

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list