[PATCH] D108699: [LAA] Analyze pointers forked by a select

[Graham Hunter via Phabricator via llvm-commits]

huntergr marked 5 inline comments as done.
huntergr added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:779
+    Type *IntPtrTy = SE->getEffectiveSCEVType(BaseExpr->getType());
+    SCEV::NoWrapFlags Wrap =
+        GEP->isInBounds() ? SCEV::FlagNSW : SCEV::FlagAnyWrap;
----------------
fhahn wrote:
> I don't think we can use the inbounds info here, unless we prove that the program is undefined if GEP is poison.
> 
> Consider something like below. If `%c` is always false, the GEP index could be out-of-bounds (and the GEP poison). Adding a runtime check based on the SCEV expression may introduce a branch on poison unconditionally.
> 
> ```
> define dso_local void @forked_ptrs_different_base_same_offset(float* nocapture readonly %Base1, float* nocapture readonly %Base2, float* nocapture %Dest, i32* nocapture readonly %Preds, i1 %c) {
> entry:
>   br label %for.body
> 
> for.cond.cleanup:
>   ret void
> 
> for.body:
>   %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %latch ]
>   %arrayidx = getelementptr inbounds i32, i32* %Preds, i64 %indvars.iv
>   %0 = load i32, i32* %arrayidx, align 4
>   %cmp1.not = icmp eq i32 %0, 0
>   %spec.select = select i1 %cmp1.not, float* %Base2, float* %Base1
>   %.sink.in = getelementptr inbounds float, float* %spec.select, i64 %indvars.iv
>   %.sink = load float, float* %.sink.in, align 4
>   %1 = getelementptr inbounds float, float* %Dest, i64 %indvars.iv
>   br i1 %c, label %then, label %latch
> 
> then:
>   store float %.sink, float* %1, align 4
>   br label %latch
> 
> latch:
>   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>   %exitcond.not = icmp eq i64 %indvars.iv.next, 100
>   br i1 %exitcond.not, label %for.cond.cleanup, label %for.body
> }
> ```
I removed the code to add no-wrap flags to the SCEV based on 'inbounds', then added your test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108699