[PATCH] D57598: [VPLAN] Determine Vector Width programmatically.

[Nikolay Panchenko via Phabricator via llvm-commits]

npanchen added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1001
   InstWidening getWideningDecision(Instruction *I, unsigned VF) {
-    assert(VF >= 2 && "Expected VF >=2");
-
----------------
fhahn wrote:
> Is this related to the patch? I suppose guessVectorizationFactor could set UserVF to 1, which could reach this code. It would be better to not vectorize with VF == 1.
For VF = 1 CM is not called, thus this change looks as unrelated to this commit. If it does, what is a reason and why other places in CM are not modified ?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7098
 
+unsigned guessVPlanVF(Loop &L, unsigned WidestVectorRegBits) {
+  unsigned Max = 1;
----------------
hsaito wrote:
> This is in some sense reinventing the wheel, and I kind of imagine this can quickly get out of control if different people start trying to add different constraints of their interest.
> 
> Let's start talking about how we can get to the point of being able to use unified computeMaxVF(). For this patch, however, I'd be happy if we get to the point of being able to use unified getSmallestAndWidestTypes().
Agree with Hideki. Ideally, this function has to return a range [MinVF, MaxVF], which can be used in buildVPlans() to minimize number of built VPlans.

Please, remember that target can have different vector registers.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7103
+    for (auto I = B->begin(), E=B->end(); I!=E; ++I ) {
+      if (!isa<LoadInst>(*I) && !isa<StoreInst>(*I))
+        continue;
----------------
For a cases when load and store instructions were hoisted out of the loop, this function will return WidestVectorRegBits, which could be >= 128.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7106
+
+      Type *ScalarDataTy = getMemInstValueType(&(*I));
+      unsigned Size = ScalarDataTy->getPrimitiveSizeInBits();
----------------
getMemInstValueType(I)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57598