[llvm] r272703 - [ValueTracking] Calls to @llvm.assume always return

Tue Jun 14 18:24:29 PDT 2016

Thinking about this a bit, it is more accurate to say "assume either
transfers execution to its successor in all well-defined programs".
In this way, its behavior is like an udiv, sdiv or a normal load
instruction (you could even "formalize" assume(x) as being equivalent
to "(void)(1 `udiv` x)" or "{int a[1]; (void)a[!x];}").

So if you have:

volatile store i32 0, i32* %p
call void @llvm.assume(i1 false)

and are interpreting the volatile store as "{ print('Thank you for
playing Wing Commander'); exit(0); }" (i.e. some arbitrary side
effect, may exit the program) then reordering the assume to before the
volatile store is incorrect, for the same reason as reordering a load
from a potentially non-dereferenceable memory location would be -- you
could turn a well defined program into an undefined program.

If you're interpreting the volatile store as always terminating then
the program is undefined to begin with, since it executes
"assume(false)".

The tail merging example okay, since we know that the volatile store
"returns" then we have undefined behavior for any value of %cond, so
in the tail merged result, we can put whatever we want after the
volatile store (since the volatile store cannot "return" for any well
defined program).

-- Sanjoy

On Tue, Jun 14, 2016 at 5:12 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
>
>
> On Tue, Jun 14, 2016 at 1:58 PM, Sanjoy Das <sanjoy at playingwithpointers.com>
> wrote:
>>
>> On Tue, Jun 14, 2016 at 1:48 PM, David Majnemer
>> <david.majnemer at gmail.com> wrote:
>> > Hmm, what if we have:
>> >
>> > volatile store i32 0, i32* %p
>> > call void @llvm.assume(i1 false)
>> > ...
>> >
>> > Can't this sort of logic let us sink the store past the assume? What if
>> > the
>> > assume gets replaced with unreachable?
>>
>> I think that's fine.  assume(X) does not mean "the control flow can
>> move past the assume only if X is true", it means "due to some
>> external invariant I know that X is true".  So in the above example,
>> we know that the volatile store itself is UB since it postdominates
>> assume(false).
>
>
>
> Hmm.  What about tail merging:
>
> head:
>   br i1 %cond, label %t, %f
>
> t:
>   store volatile i32 0, i32* %p
>   unreachable
>
> f:
>   store volatile i32 0, i32* %p
>   call void @llvm.assume(i1 false)
>
> into:
>
> head:
>   store volatile i32 0, i32* %p
>   call void @llvm.assume(i1 false)
>
> ?
>
>>
>>
>> -- Sanjoy
>>
>> >
>> >
>> > On Tue, Jun 14, 2016 at 1:33 PM, Sanjoy Das via llvm-commits
>> > <llvm-commits at lists.llvm.org> wrote:
>> >>
>> >> Forgot to mention in the commit message: this regression was
>> >> introduced in http://reviews.llvm.org/rL272489
>> >>
>> >> On Tue, Jun 14, 2016 at 1:23 PM, Sanjoy Das via llvm-commits
>> >> <llvm-commits at lists.llvm.org> wrote:
>> >> > Author: sanjoy
>> >> > Date: Tue Jun 14 15:23:16 2016
>> >> > New Revision: 272703
>> >> >
>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=272703&view=rev
>> >> > Log:
>> >> > [ValueTracking] Calls to @llvm.assume always return
>> >> >
>> >> > This change teaches llvm::isGuaranteedToTransferExecutionToSuccessor
>> >> > that calls to @llvm.assume always terminate.  Most other relevant
>> >> > intrinsics should be covered by the "CS.onlyReadsMemory() ||
>> >> > CS.onlyAccessesArgMemory()" bit but we were missing @llvm.assumes
>> >> > because we state that it clobbers memory.
>> >> >
>> >> > Added an LICM test case, but this change is not specific to LICM.
>> >> >
>> >> > Modified:
>> >> >     llvm/trunk/lib/Analysis/ValueTracking.cpp
>> >> >     llvm/trunk/test/Transforms/LICM/assume.ll
>> >> >
>> >> > Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=272703&r1=272702&r2=272703&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
>> >> > +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Tue Jun 14 15:23:16
>> >> > 2016
>> >> > @@ -3478,7 +3478,8 @@ bool llvm::isGuaranteedToTransferExecuti
>> >> >      // but it's consistent with other passes. See
>> >> > http://llvm.org/PR965
>> >> > .
>> >> >      // FIXME: This isn't aggressive enough; a call which only writes
>> >> > to
>> >> > a
>> >> >      // global is guaranteed to return.
>> >> > -    return CS.onlyReadsMemory() || CS.onlyAccessesArgMemory();
>> >> > +    return CS.onlyReadsMemory() || CS.onlyAccessesArgMemory() ||
>> >> > +           match(I, m_Intrinsic<Intrinsic::assume>());
>> >> >    }
>> >> >
>> >> >    // Other instructions return normally.
>> >> >
>> >> > Modified: llvm/trunk/test/Transforms/LICM/assume.ll
>> >> > URL:
>> >> >
>> >> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LICM/assume.ll?rev=272703&r1=272702&r2=272703&view=diff
>> >> >
>> >> >
>> >> > ==============================================================================
>> >> > --- llvm/trunk/test/Transforms/LICM/assume.ll (original)
>> >> > +++ llvm/trunk/test/Transforms/LICM/assume.ll Tue Jun 14 15:23:16
>> >> > 2016
>> >> > @@ -1,6 +1,7 @@
>> >> >  ; RUN: opt -licm -basicaa < %s -S | FileCheck %s
>> >> >
>> >> > -define void @f(i1 %p) nounwind ssp {
>> >> > +define void @f_0(i1 %p) nounwind ssp {
>> >> > +; CHECK-LABEL: @f_0(
>> >> >  entry:
>> >> >    br label %for.body
>> >> >
>> >> > @@ -31,4 +32,20 @@ for.end104:
>> >> >    ret void
>> >> >  }
>> >> >
>> >> > +define void @f_1(i1 %cond, i32* %ptr) {
>> >> > +; CHECK-LABEL: @f_1(
>> >> > +; CHECK: %val = load i32, i32* %ptr
>> >> > +; CHECK-NEXT:  br label %loop
>> >> > +
>> >> > +entry:
>> >> > +  br label %loop
>> >> > +
>> >> > +loop:
>> >> > +  %x = phi i32 [ 0, %entry ], [ %x.inc, %loop ]
>> >> > +  call void @llvm.assume(i1 %cond)
>> >> > +  %val = load i32, i32* %ptr
>> >> > +  %x.inc = add i32 %x, %val
>> >> > +  br label %loop
>> >> > +}
>> >> > +
>> >> >  declare void @llvm.assume(i1)
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > llvm-commits mailing list
>> >> > llvm-commits at lists.llvm.org
>> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >>
>> >>
>> >>
>> >> --
>> >> Sanjoy Das
>> >> http://playingwithpointers.com
>> >> _______________________________________________
>> >> llvm-commits mailing list
>> >> llvm-commits at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >
>> >
>>
>>
>>
>> --
>> Sanjoy Das
>> http://playingwithpointers.com
>
>

-- 
Sanjoy Das
http://playingwithpointers.com