[PATCH] D20495: [BasicAA] An inbounds GEP and an alloca can't alias if the base of the GEP would point "below" the alloca

[Xinliang David Li via llvm-commits]

On Tue, May 24, 2016 at 3:14 PM, Michael Kuperstein <mkuper at google.com>
wrote:

> mkuper added inline comments.
>
> ================
> Comment at: lib/Analysis/BasicAliasAnalysis.cpp:998
> @@ -997,3 @@
> -        // assert.
> -        if (GEP1BasePtr != UnderlyingV1 || GEP2BasePtr != UnderlyingV2) {
> -          return MayAlias;
> ----------------
> davidxl wrote:
> > mkuper wrote:
> > > davidxl wrote:
> > > > Extract this unrelated cleanup into  separate patch.
> > > It's not exactly unrelated.
> > >
> > > The old code would call DecomposeGEPExpression on-demand only in code
> paths that were using the decomposition. Which was, I think, the majority -
> but not 100%. I needed to hoist the calls up - but I don't think that
> should go in as a stand-alone patch, since if it goes in without the rest
> of my changes, it incurs compile-time cost without much benefit.
> > >
> > > If you think the clean-up is worth that, I can split it into a
> separate patch, but I'd rather not.
> > I mean the part that turns return MayAlias into assert -- not the
> hoisting part of the change. I think you just need to make that change
> (lgtm to that part which can be committed first) and rebase this patch. Or
> perhaps just leaves the FIXME in this patch (in the hoisted code and clean
> it up as a follow up.
> Oh, yes, that makes perfect sense, but is slightly more annoying than it
> looks.
>
> In fact, I already committed that as a separate patch a few days ago
> (r270268) - but reverted it because of unused variable warnings in
> non-asserts builds. I thought about recommitting it with dummy accesses to
> silence the warnings, but that seemed silly. (And I can't put the calls
> under NDEBUG, because only the result of the call is unused, but the output
> parameters aren't).
>
> Leaving the FIXME in this patch could potentially change behavior, since
> it would return MayAlais on different code paths, even those that
> originally didn't care about the bound at all. On the other hand, it should
> never actually fire, so that should be fine.
>
> I'm not too excited about either option, but if you prefer one of them to
> the whole thing going in in one piece, let me know which one.
>
>
Ah -- I see -- you have already gone though exercise  -- so it is fine to
leave it here.

David


>
> http://reviews.llvm.org/D20495
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160524/49a9b60b/attachment.html>