[PATCH] D115873: [LAA] Add remarks for unbounded array access

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:824
           CanDoAliasSetRT = false;
+          if (!UncomputablePtr)
+            UncomputablePtr = Access.getPointer();
----------------
I believe this check should be removed (see my other comment as well)


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:2110
       auto *SE = PSE->getSE();
-      CanDoRTIfNeeded = Accesses.canCheckPtrAtRT(*PtrRtChecking, SE, TheLoop,
-                                                 SymbolicStrides, true);
+      Value *UncomputablePtr;
+      CanDoRTIfNeeded = Accesses.canCheckPtrAtRT(
----------------
This variable aliases with `UncomputablePtr` defined on line 2075. Either pass in the same variable, or give this variable a different name.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:565
+                       bool ShouldCheckWrap = false,
+                       Value **UncomputablePtr = nullptr);
 
----------------
malharJ wrote:
> malharJ wrote:
> > sdesmalen wrote:
> > > Instead of passing in the pointer with a default value of `nullptr`, can you make it a reference, so that it always gets set, e.g.
> > > 
> > >   Value *&UncomputablePtr
> > > 
> > > That also removes the need for the conditional write.
> > Can we please leave it as initialised as nullptr ?
> > 
> > The conditional write ensures that the first unsafe dependence gets reported by the remark.
> > 
> > 
> > ```
> >  if (UncomputablePtr)
> >      *UncomputablePtr = Access.getPointer();
> > ```
> > 
> > 
> > 
> > 
> Ok please ignore the above. I've made the change as suggested,
> 
> But I didn't remove that bit in the code.
> The conditional write ensures that the first unsafe dependence gets reported by the remark.
> 
> ```
> if (!UncomputablePtr)
>     UncomputablePtr = Access.getPointer();
> ```
> 
> But I didn't remove that bit in the code.
> The conditional write ensures that the first unsafe dependence gets reported by the remark.
That should be the choice of the caller, not the callee. If the caller doesn't want the value to be overwritten, it should pass a dummy variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115873