[PATCH] D106099: [DependenceAnalysis] Guard analysis using getPointerBase()

[Artem Radzikhovskyy via Phabricator via llvm-commits]

artemrad added inline comments.


================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:3556
   LLVM_DEBUG(dbgs() << "    DstSCEV = " << *DstSCEV << "\n");
+  if (SE->getPointerBase(SrcSCEV) != SE->getPointerBase(DstSCEV)) {
+    // If two pointers have different bases, trying to analyze indexes won't
----------------
efriedma wrote:
> artemrad wrote:
> > This seems like a subset of cases we should be filtering out. Please also add checks for the access width being unaligned with the store/load size. In this case the base pointers are different but the accesses can still overlap. This case will not be caught by DependenceAnalysis. 
> > 
> > 
> > ```
> > define void @z0(i32* %A, i64 %n) nounwind uwtable ssp {
> > entry:
> >   %arrayidx = getelementptr inbounds i32, i32* %A, i64 %n
> >   %arrayidx_cast = bitcast i32* %arrayidx to i64*
> >   store i64 0, i64* %arrayidx_cast, align 4
> > 
> > ; CHECK: da analyze - confused!
> > ; CHECK: da analyze - confused!
> > ; CHECK: da analyze - confused!
> > ; CHECK: da analyze - none
> > ; CHECK: da analyze - confused!
> > ; CHECK: da analyze - confused!
> > 
> >   %add1 = add i64 %n, 1
> >   %arrayidx2 = getelementptr inbounds i32, i32* %A, i64 %add1
> >   %0 = load i32, i32* %arrayidx2, align 4
> >  
> >   ; Even if the load and store types are the same, if their
> >   ; alignments are smaller than the store size and they are
> >   ; accesses through something other than a straightforward
> >   ; gep, they can still overlap, as this access shows
> >   %arrayidx2_cast = bitcast i32* %arrayidx2 to i64*
> >   %1 = load i64, i64* %arrayidx2_cast, align 4
> >   ret void
> > }
> > ```
> > 
> > 
> > 
> > 
> Well, there are a few things we can do in that case:
> 
> 1. Bail out if we don't have two naturally aligned accesses of the same size.
> 2. Perform multiple dependency checks, and merge the results.  For example, if you have a size 4 access, and a size 8 access, you can treat the size 8 access as two size 4 accesses, and merge the results accordingly.
> 3. Teach the dependence checks to use ranges, instead of plain indexes.
> 
> I'm afraid (1) will hurt performance. And I don't really want to sign up to do (2) or (3).
Well currently we do not bail out in the case of (1), so it is worse than loosing performance, we are giving incorrect results. 

As for (2) and (3) completely agree this is a project and I did not mean to imply that you should undertake it. Apologies, that was not my intention. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106099