[PATCH] D50778: [LV] Vectorize loops where non-phi instructions used outside loop

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3727
+      if (isa<Instruction>(IncomingValue)) 
+          LastLane = Cost->isUniformAfterVectorization(
+                         cast<Instruction>(IncomingValue), VF)
----------------
Ayal wrote:
> anna wrote:
> > I have trouble getting a test case that exercises this path. Tried couple of different uniform instructions (GEPs with constant operands). In all cases, we were vectorizing these and then extracting the VF -1 th element. 
> > Any ideas on what will be a good test case for this? Thanks.
> > 
> Yes, the GEPs themselves are immune, because they are checked by `UsersAreMemAccesses()` to see if all their users are loads/stores, so if they have a loop-closed phi as a user they will not be regarded as uniform. The following should work:
> 
> ```
> ; RUN: opt < %s -loop-vectorize -S
> 
> target datalayout = "E-m:e-p:32:32-i64:32-f64:32:64-a:0:32-n32-S128"
> 
> @tab = common global [32 x i8] zeroinitializer, align 1
> 
> define i32 @uniform_live_out() {
> entry:
>   br label %for.body
> 
> for.body:                                         ; preds = %for.body, %entry
>   %i.08 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
>   %i.09 = add i32 %i.08, 7
>   %arrayidx = getelementptr inbounds [32 x i8], [32 x i8]* @tab, i32 0, i32 %i.09
>   %0 = load i8, i8* %arrayidx, align 1
>   %bump = add i8 %0, 1
>   store i8 %bump, i8* %arrayidx, align 1
>   %inc = add nsw i32 %i.08, 1
>   %exitcond = icmp eq i32 %i.08, 20000
>   br i1 %exitcond, label %for.end, label %for.body
> 
> for.end:                                          ; preds = %for.body
>   %lcssa = phi i32 [%i.09, %for.body]
>   %arrayidx.out = getelementptr inbounds [32 x i8], [32 x i8]* @tab, i32 0, i32 %lcssa
>   store i8 42, i8* %arrayidx.out, align 1
>   ret i32 0
> }
> ```
> 
> Sorry about this. It's really due to the way uniform values are stored. Would be better to have an API for obtaining the scalar value corresponding to the last VF-1 lane, regardless of how it's stored, instead of having users check if VF-1 or 0 should be passed.
> 
> But wait, this test shows that `LoopVectorizationCostModel::collectLoopUniforms()` must be updated too, to refrain from recognizing instructions with live-outs as being uniform; except for instructions that are inherently uniform/invariant (i.e., all VF values will be the same), and induction variables with their bumps - which are pre-computed rather than have their last-iteration value taken.
thanks Ayal. I've made the change in `collectLoopUniforms` to avoid incorrectly recognizing instructions with outside uses as uniform (unless those uses themselves were uniform). So, `%i.09 = add i32 %i.08, 7` is no longer scalarized and we extract the VF-1th element. Without that change, we were recognizing i.09 as uniform. 
I will add this test case and find another one where the live-out is of a really uniform instruction.


Repository:
  rL LLVM

https://reviews.llvm.org/D50778