[PATCH] D19730: [BasicAA] Treat llvm.assume as not accessing memory in getModRefBehavior(Function)

[Daniel Berlin via llvm-commits]

On Fri, Apr 29, 2016 at 10:57 AM, Hal Finkel <hfinkel at anl.gov> wrote:

>
> ------------------------------
>
> *From: *"Daniel Berlin" <dberlin at dberlin.org>
> *To: *"Hal Finkel" <hfinkel at anl.gov>
> *Cc: *"Geoff Berry" <gberry at codeaurora.org>, "Chandler Carruth" <
> chandlerc at gmail.com>, "Philip Reames" <listmail at philipreames.com>,
> "Sanjoy Das" <sanjoy at playingwithpointers.com>, mcrosier at codeaurora.org,
> "llvm-commits" <llvm-commits at lists.llvm.org>, "Junbum Lim" <
> junbuml at codeaurora.org>,
> reviews+D19730+public+66d50dddcf8372b8 at reviews.llvm.org
> *Sent: *Friday, April 29, 2016 12:31:28 PM
> *Subject: *Re: [PATCH] D19730: [BasicAA] Treat llvm.assume as not
> accessing memory in getModRefBehavior(Function)
>
>
>>
>> I'm not sure this is a good idea. As the comment says, they "never
>> aliases any particular memory location". This, however, is not comparing
>> against any particular location.
>
>
> It is asking for general behavior.
>
> Yes, and it neither mod nor refs anything :)
>
>
>> AA should probably be constant with the function attributes here.
>>
>
> BasicAA already was working around this.
>
> I know, I'm responsible for the workarounds.
>
;)

>
> This just puts this knowledge in one place instead of two (basicaa and
> memoryssa).
>
> In any case, if we are going to do that, we'd have to actually fix things
> properly.  Saying assume has side-effects and writes things was wrong in
> the first place compared to actually modeling control dependencies for
> things like assume properly.
>
> It simply does not have side-effects or write anything. Period.  Claiming
> otherwise is a lie to try to avoid fixing other infrastructure to handle
> this kind of intrinsic properly.
>
> Until we go and have the wherewithal to fix that infrastructure, we are
> going to have to accept that we are going to have things like this, where
> we are making up for it.
>
> I'm well aware of the state of things, and I agree that we should have a
> better way of modeling control dependencies than via memory.
>
>
> The alternative, which was currently done, where basicaa and memoryssa and
> whoever else has to know that assume is magic and special, seems much worse.
>
> I understand. However, without our current system, this still obviously
> needs to be correct. What is to prevent a pass from (correctly) seeing the
> call to assume, using AA to conclude it does not actually alter memory,
> seeing it has no return values, intrinsics don't throw, and so removing it
> as dead?
>

Well, it will still return true to mayHaveSideEffects :)

But your question is exactly one of "how do we fix the infrastructure".
I don't think changing AA, which is an optional part and supposed to give
the best answers it can (regardless of function attributes or anything
else), is going to affect that much.

As an aside, note that this is already affecting things like GlobalsModRef
(which also mixes AA and function attributes), which will now assume that
every function in a callchain with an assume in it modref things (which is
making much worse information :( ).

It too would have to special case assume to tell it otherwise.

This patch should also fix part of this without breaking assume (the other
part it will not due to the mixing of AA and function attribute calls).

However, I'm also happy to put this knowledge in the 4 places for now if
you really believe that is a better solution.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160429/ccf03361/attachment.html>