[LLVMdev] where should volatileness really live in the AA API?

[Daniel Berlin]

On Thu, Apr 16, 2015 at 6:31 PM, Philip Reames
<listmail at philipreames.com> wrote:
> On 03/31/2015 03:41 PM, Daniel Berlin wrote:
>>
>> So, i'm working through the last of miniscule gvn optimizations, and
>> one of the things that gets tested is whether it will eliminate
>> non-volatile loads in favor of ohter non-volatile loads when there are
>> volatile loads in the way.
>
> I'm pretty sure I'm the one who added that set of test cases.  :)
>>
>> I.E.
>> define i32 @test1(i32* nocapture %p, i32* nocapture %q) {
>> entry:
>>    %x = load i32, i32* %p
>>    %0 = load volatile i32, i32* %q
>>    %y = load i32, i32* %p
>>    %add = sub i32 %y, %x
>>    ret i32 %add
>> }
>>
>>
>> Currently, getModRefInfo will return Mod for getModRefInfo(%0,
>> AA->getLocation(%y))
>>
>> This is because it punts on saying anything about ordered loads
>
> I think this is a conservatively correct, but non ideal answer.  A correct
> answer would also be to return Ref or NoModRef (depending on actual aliasing
> of the underlying locations).  Ordering is not an alasing property and
> should not be reported as such.  However, I believe a large number of
> callers may expect this and fleshing out all those bugs may not be fun.

Yeah, I realized about 10 minutes after writing that email that
including it in location would be a bad idea, and that they should be
reported as different properties.

I do think getModRefInfo is wrong here, given what it knows. It does
*not* modify memory, and it knows it.
The fact that it is volatile may make it a dependency for another
load, but that has no relevance to whether it modifies memory or not.

I'd rather flesh out these bugs, and report the, IMHO, right answer.

>
> Also worth commenting on is that *volatile* and *ordering* have distinctly
> different properties.  We couple them in a lot of odd ways, but the
> reasoning and legal optimizations are quite different.
>
>> I can certainly watch for this case in the caller, and ignore it if
>> it's volatile load vs non-volatile load. This is what MemDep does.
>
> Can you point to the code you're talking about?  A volatile load encountered
> during the normal instruction walk is treated like any other (provided we
> know the query instruction is non-volatile.)

Yes, this is what i meant by "it ignores it". It decided is had no
impact as a dependency.

>  We are conservative about
> volatile stores, but that's on my list to fix.
>
> Note that we are really conservative about volatile query instructions

>> Nothing that involves walking, sure, but it can trivially give the
>> same answer to this case, which is "the volatile load does not affect
>> the regular load".
>
> I strongly believe that ordering and aliasing are distinct properties.
>

Which is fine, so we should stop reporting one as if it affects the other.