[PATCH] D42381: [DA] Correct size parameter from dependency analysis to AA

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: lib/Analysis/DependenceAnalysis.cpp:629
   const Value *BObj = GetUnderlyingObject(B, DL);
-  return AA->alias(AObj, DL.getTypeStoreSize(AObj->getType()),
-                   BObj, DL.getTypeStoreSize(BObj->getType()));
+  return AA->alias(AObj, BObj);
 }
----------------
nlopes wrote:
> hfinkel wrote:
> > > It's possible that the size here should be the size of the thing that AObj points to, as opposed to unknown
> > 
> > There are two problems here, and I think that we can fix them both. What we're really trying to do here is to identify disjoint underlying objects.
> > 
> >  * First, GetUnderlyingObject might not always return the underlying object (because it has a recursion-depth cutoff). Thus, we need to validate the fact that GetUnderlyingObject actually did return such a thing. So we really want to do:
> > 
> > 
> >   if (!isIdentifiedObject(AObj) || !isIdentifiedObject(BObj))
> >     return true;
> > 
> > 
> >  * At that point, the sizes are essentially irrelevant. and we should pass MemoryLocation::UnknownSize as the size. However, at that point, since we have unique underlying objects, we can just compare them, so we can just do (there's no further value in calling into AA):
> > 
> > 
> >   return AObj == BObj;
> > 
> > 
> I agree with Hal's comment. Seems like the way to go.
This sounds good. I like the simplification. I've updated things to something that may still be wrong.

As far as I understand, this is returning a tristate:
 MustAlias - Do dependency analysis.
 NoAlias - No dependence is possible.
 MayAlias - Don't know anything - a confused dependence.

A lot of the tests use function arguments as input (as will real code). If AObj and BObj are the same argument, we are obviously mustalias, even if that isn't an identified object. So I think this is now still correct even if we hit the recursion limit, the mustalias will still be valid.

Also (and this might be something that doesn't come up very often) what if one argument has a noalias attribute, but another doesn't (or one is a alloca, the other an argument). We can still prove noalias there?

Also I was hoping to get TBAA working here. See D42382. As in - if the loads/stores of the Src and Dst are different types, we know there is no alias, so no dependency. Any ideas what the best bet there would be? Call alias on the Src and Dst? Work with the TBAA metadata somehow? (I'm not sure that's possible)


https://reviews.llvm.org/D42381