[llvm-dev] A bug related with undef value when bootstrap MemorySSA.cpp

[Sanjoy Das via llvm-dev]

 Hi,

On Mon, Jul 17, 2017 at 10:32 AM, Xinliang David Li <davidxl at google.com> wrote:
> The issue blocks another optimization patch and Wei has spent huge amount of
> effort isolating the the bootstrap failure to this same problem. I agree
> with Wei that other developers may also get hit by the same issue and the
> cost of leaving this issue open for long can be very high to the community.

I can't speak for others, but I am fine with adding a workaround for
this.  However, I suspect isGuaranteedNotToBeUndefOrPoison in
LoopUnswitch may regress other benchmarks.

Thanks!
-- Sanjoy

>
> David
>
> On Mon, Jul 17, 2017 at 10:01 AM, Wei Mi <wmi at google.com> wrote:
>>
>> Sanjoy and Nuno, thanks for pointing me to the existing bug and the
>> related discussion, and thanks for the great effort to try to solve it
>> fundamentally in IR.
>>
>> I tried the patches and it compiled the small case and also
>> MemorySSA.cpp correctly.
>>
>> I wish we can have the patches in ASAP but they are some big patches
>> and I guess it still takes some time to get in. The issue when
>> compiling MemorySSA.cpp can be easily exposed by other changes, and it
>> is very time consuming to triage and extract the issue, so we like to
>> have some temporary solution, like to make loop unswitching more
>> conservative. If it sounds ok, I can work on it. My plan is to enhance
>> isGuaranteedNotToBeUndefOrPoison and use it as a precondition in loop
>> unswitching.
>>
>> Thanks,
>> Wei.
>>
>>
>>
>> On Mon, Jul 17, 2017 at 1:24 AM, Nuno Lopes <nlopes at microsoft.com> wrote:
>> > Cool, thanks for debugging this issue and letting us know.
>> >
>> > We have a few patches to fix this issue:
>> >  - Introduce freeze in IR: https://reviews.llvm.org/D29011
>> >  - Lowering freeze: https://reviews.llvm.org/D29014
>> >  - Fix loop unswitch: https://reviews.llvm.org/D29015
>> >
>> > Bonus patches to recover perf:
>> >  - Be less conservative in loop unswitching:
>> > https://reviews.llvm.org/D29016
>> >  - Instcombine support for freeze: https://reviews.llvm.org/D29013
>> >
>> > It would be great if you could try out these patches to see if the bug
>> > goes away.
>> >
>> > Thanks,
>> > Nuno
>> >
>> >
>> > -----Original Message-----
>> > From: Sanjoy Das
>> > Sent: 16 de julho de 2017 02:47
>> > To: Wei Mi
>> > Cc: llvm-dev; Xinliang David Li; Sanjoy Das; Nuno Lopes; John Regehr;
>> > Juneyoung Lee
>> > Subject: Re: [llvm-dev] A bug related with undef value when bootstrap
>> > MemorySSA.cpp
>> >
>> > Hi Wei,
>> >
>> > [+CC the other undef folks]
>> >
>> > This is the same bug as
>> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.llvm.org%2Fshow_bug.cgi%3Fid%3D31652&data=02%7C01%7Cnlopes%40microsoft.com%7C971ae2063c8b420d2f1008d4cbec8f22%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636357664258073352&sdata=iCy8kUlWwJzQpXbXvOuhHpKyXBnntZPGvn%2FtxWsqun8%3D&reserved=0.
>> > The short answer here is that the loop unswitch transform and the GVN
>> > transform are justified via conflicting specifications of undef (that is,
>> > LoopUnswitch and GVN don't agree on the definition of undef).  The
>> > long(er) answer is in the bug.
>> >
>> > Unfortunately, there is no real way to fix this in the IR today (beyond
>> > hacking GVN / LoopUnswitch to back off on these transforms enough to not
>> > trigger this case).  We will need to specify and implement a consistent
>> > definition of undef / poison to truly fix this, one of which we've already
>> > proposed.
>> >
>> > Thanks!
>> > -- Sanjoy
>
>