llvm dev - Jun 2016 - Early CSE clobbering llvm.assume

As of llvm 3.8, the early CSE pass seems to remove llvm.assume intrinsics.
Is this the expected behavior?

I've attached as small-ish example of this happening in my production code.

    $ opt -early-cse before-early-cse.ll -S > after-early-cse.ll

Note the use of the assume intrinsic indicating that the loaded value
%channels equals 3. In a later pass I replace the load instruction with the
constant value. This approach worked in llvm 3.7. I can solve the issue by
moving my pass before early CSE if need be.

v/r,
Josh
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/0498933f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: after-early-cse.ll
Type: application/octet-stream
Size: 3903 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/0498933f/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: before-early-cse.ll
Type: application/octet-stream
Size: 4615 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/0498933f/attachment-0001.obj>

On second thought, I don't have a workaround. I had been relying on
EarlyCSE + GVN to combine %channels and %channels4, which I would then
replace with the constant 3. If I run my optimization before these passes
then I have no way of knowing %channels4 can be replaced with the constant
3.

On Fri, Jun 10, 2016 at 11:42 AM, Josh Klontz <josh.klontz at gmail.com>
wrote:
> As of llvm 3.8, the early CSE pass seems to remove llvm.assume intrinsics.
> Is this the expected behavior?
>
> I've attached as small-ish example of this happening in my production
code.
>
>     $ opt -early-cse before-early-cse.ll -S > after-early-cse.ll
>
> Note the use of the assume intrinsic indicating that the loaded value
> %channels equals 3. In a later pass I replace the load instruction with the
> constant value. This approach worked in llvm 3.7. I can solve the issue by
> moving my pass before early CSE if need be.
>
> v/r,
> Josh
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/035066d4/attachment.html>

Thanks for the lead Kevin. Unfortunately when I updated to ToT the problem
persists. Will put together a minimum reproducing example.

On Fri, Jun 10, 2016 at 12:26 PM, Smith, Kevin B <kevin.b.smith at
intel.com>
wrote:
> You might look at this commit to fix the problem: r270823
> "MemorySSA: Revert r269678 and r268068; replace with special casing in
> MemorySSA."
>
> I think that might fix the issue for you.
>
> Kevin Smith
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/cb5b611d/attachment.html>

Here's a minimum reproducing example (attached and inline below):

define i32 @foo(i32*) {
entry:
  %1 = load i32, i32* %0
  %2 = icmp eq i32 %1, 3
  call void @llvm.assume(i1 %2)
  %3 = load i32, i32* %0
  ret i32 %3
}

Both -early-cse and -gvn independently simplify this IR to:

define i32 @foo(i32*) {
entry:
  %1 = load i32, i32* %0
  ret i32 %1
}

Whereas I think it would be more correct to simplify it to:

define i32 @foo(i32*) {
entry:
  %1 = load i32, i32* %0
  %2 = icmp eq i32 %1, 3
  call void @llvm.assume(i1 %2)
  ret i32 %1
}

And then:

define i32 @foo(i32*) {
entry:
  ret i32 3
}

Both -early-cse and -gvn appear capable of this final optimization, but get
stuck doing the wrong thing when given the initial code with the redundant
load.

v/r,
Josh

On Fri, Jun 10, 2016 at 11:42 AM, Josh Klontz <josh.klontz at gmail.com>
wrote:
> As of llvm 3.8, the early CSE pass seems to remove llvm.assume intrinsics.
> Is this the expected behavior?
>
> I've attached as small-ish example of this happening in my production
code.
>
>     $ opt -early-cse before-early-cse.ll -S > after-early-cse.ll
>
> Note the use of the assume intrinsic indicating that the loaded value
> %channels equals 3. In a later pass I replace the load instruction with the
> constant value. This approach worked in llvm 3.7. I can solve the issue by
> moving my pass before early CSE if need be.
>
> v/r,
> Josh
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/d881d19d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: assume_constant.ll
Type: application/octet-stream
Size: 318 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/d881d19d/attachment.obj>

Yeah, that change is completely unrelated, that is about correctness, this
is about optimization.
I'm working on a proposal to just fix assume at some point to deal with the
former issue.

The problem with this testcase is that all the ways assume is propagate
expect the variable in the assume to later be used.


<This is the main way assume constants are propagated>
bool GVN::processAssumeIntrinsic(IntrinsicInst *Inst) {
...

   // We can replace assume value with true, which covers cases like this:
   // call void @llvm.assume(i1 %cmp)
   // br i1 %cmp, label %bb1, label %bb2 ; will change %cmp to true
   ReplaceWithConstMap[V] = True;
...
}

...
bool GVN::processBlock(BasicBlock *BB) {
...
   // Clearing map before every BB because it can be used only for single
BB.
   ReplaceWithConstMap.clear();
....

}


So it's going to go through the rest of the bb, see nothing with %2, do
nothing, and then next iteration, clear the constant map.  It's not valid
to avoid clearing the constant map, and in fact,  what is happening here is
a pretty complicated to help with.
There is no easy way to see that the load  at %3 is affected at all by the
assume.

It's possible to make this work using the predication/value inference
algorithms in the paper newgvn is based on, but it's not even implemented
there.

Short answer, without special casing this in magic ways, i wouldn't expect
this to get fixed anytime soon.

If we fixed assume in one of the ways i thought about, like bodik's
extended ssa:
http://homepages.dcc.ufmg.br/~fernando/classes/dcc888/ementa/slides/RangeAnalysis.pdf

You would at least see that the load result is used by an assume, and could
go look at that assume and so something with it.  Currently, it's a few
steps away.

In the current scheme, assumes just float in air, and so it can be hard to
see what their effects touch
:)


On Fri, Jun 10, 2016 at 2:00 PM, Josh Klontz via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Thanks for the lead Kevin. Unfortunately when I updated to ToT the problem
> persists. Will put together a minimum reproducing example.
>
> On Fri, Jun 10, 2016 at 12:26 PM, Smith, Kevin B <kevin.b.smith at
intel.com>
> wrote:
>
>> You might look at this commit to fix the problem: r270823
>> "MemorySSA: Revert r269678 and r268068; replace with special
casing in
>> MemorySSA."
>>
>> I think that might fix the issue for you.
>>
>> Kevin Smith
>>
>>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/f22e2b42/attachment.html>