[PATCH] D12842: [LoopVectorization] Add an API to make the LoopVectorizer publically accessible

[Adam Nemet via llvm-commits]

anemet added a comment.

Hi James,

Can you please explain the plan here?  I am a bit confused how interleaving is now done as part of LoopVectorization::vectorize even though you create a new parallel class LoopInterleaving.  What is the plan here?

Besides, I also have some more questions and nitpicks below but probably this first question is the most critical one.

Thanks,
Adam

I am assuming this is NFC.  Please mention that for each patch.

Do you have plans to move out the implementation to its own module?  If not immediately, you may want to explain the difficulty in the header or somewhere.


================
Comment at: include/llvm/Transforms/Utils/LoopWidening.h:60
@@ +59,3 @@
+protected:
+  void AddRuntimeUnrollDisableMetaData(Loop *L);
+  
----------------
s/Add/add

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1609-1611
@@ +1608,5 @@
+                                     bool AlwaysVectorize) :
+  SE(SE), LI(LI), DT(DT), TTI(TTI), LAA(LAA), AA(AA), AC(AC), BFI(BFI),
+  TLI(TLI),
+  DisableUnrolling(DisableUnrolling), AlwaysVectorize(AlwaysVectorize) {
+}
----------------
This does not look like properly clang-formatted  

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1614-1618
@@ +1613,7 @@
+
+bool LoopVectorization::vectorize(Loop *L,
+                                  int VectorWidth, int UnrollFactor) {
+  assert(L->empty() && "Only process inner loops.");
+  assert(VectorWidth == -1 && "Overriding VF not yet implemented!");
+  assert(UnrollFactor == -1 && "Overriding UF not yet implemented!");
+  
----------------
I think it's probably better to only add these arguments when they become supported.


Repository:
  rL LLVM

http://reviews.llvm.org/D12842