Xinliang David Li via llvm-dev
2017-Jul-17 17:32 UTC
[llvm-dev] A bug related with undef value when bootstrap MemorySSA.cpp
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. 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%7C72f988bf86f141af91ab2d7cd011 > db47%7C1%7C0%7C636357664258073352&sdata=iCy8kUlWwJzQpXbXvOuhHpKyXBnntZ > PGvn%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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170717/2ca25a54/attachment-0001.html>
Sanjoy Das via llvm-dev
2017-Jul-17 18:18 UTC
[llvm-dev] A bug related with undef value when bootstrap MemorySSA.cpp
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 > >
Daniel Berlin via llvm-dev
2017-Jul-17 18:21 UTC
[llvm-dev] A bug related with undef value when bootstrap MemorySSA.cpp
On Mon, Jul 17, 2017 at 11:18 AM, Sanjoy Das via llvm-dev < llvm-dev at lists.llvm.org> wrote:> 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. >Any other thoughts on a more minimal fix? Otherwise, it sounds like we can only try to find the fix that does the least damage. We can't just leave it broken given it's triggering even in llvm bootstraps now :( -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170717/04d5e434/attachment.html>
Reasonably Related Threads
- A bug related with undef value when bootstrap MemorySSA.cpp
- A bug related with undef value when bootstrap MemorySSA.cpp
- A bug related with undef value when bootstrap MemorySSA.cpp
- A bug related with undef value when bootstrap MemorySSA.cpp
- A bug related with undef value when bootstrap MemorySSA.cpp