[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-12-15 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } aqjune wrote: > neildhar wrote: > > spatel wrote: > > > MatzeB w

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-12-15 Thread David Spickett via Phabricator via cfe-commits
DavidSpickett added a comment. I bisected an arm 32 bit issue with WebKit Javascript's infinity value (https://github.com/llvm/llvm-project/issues/52669) down to this commit. Just to make people aware there is another failing case. No reproducer yet I'm still working on a way to reduce it, but

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-10-28 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } neildhar wrote: > spatel wrote: > > MatzeB wrote: > > > I believ

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-10-28 Thread Neil Dhar via Phabricator via cfe-commits
neildhar added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } spatel wrote: > MatzeB wrote: > > I believe this is causing so

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-10-28 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } MatzeB wrote: > I believe this is causing some of our clients tr

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-10-26 Thread Matthias Braun via Phabricator via cfe-commits
MatzeB added inline comments. Comment at: llvm/lib/IR/ConstantFold.cpp:633 // the input constant. -return UndefValue::get(DestTy); +return PoisonValue::get(DestTy); } I believe this is causing some of our clients trouble, especiall

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-02-03 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2538713 , @RKSimon wrote: > https://bugs.llvm.org/show_bug.cgi?id=49005 seems to be due to this (either > directly or it has unearthed an existing problem) I reverted this commit; I'll reland this after the underlying pr

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-02-03 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added a comment. https://bugs.llvm.org/show_bug.cgi?id=49005 seems to be due to this (either directly or it has unearthed an existing problem) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92270/new/ https://reviews.llvm.org/D92270 __

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-07 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2483557 , @thakis wrote: > It turned out to be UB in our code as far as I can tell, see > https://bugs.chromium.org/p/angleproject/issues/detail?id=5500#c36 and the > follow-on. > > (If this is known to trigger an existi

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-06 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. It turned out to be UB in our code as far as I can tell, see https://bugs.chromium.org/p/angleproject/issues/detail?id=5500#c36 and the follow-on. (If this is known to trigger an existing bug more often, it might still be a good idea to undo it until that existing bug i

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-06 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. To fix the old bug quite a few patches in LLVM have landed so far and it is still ongoing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92270/new/ https://reviews.llvm.org/D92270 __

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-06 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2482741 , @thakis wrote: > We bisected a test failure to this > (https://bugs.chromium.org/p/angleproject/issues/detail?id=5500#c17). Can you > expand a bit on what this patch means in practice? I'm guessing it makes UB

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2021-01-06 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. We bisected a test failure to this (https://bugs.chromium.org/p/angleproject/issues/detail?id=5500#c17). Can you expand a bit on what this patch means in practice? I'm guessing it makes UB in C++ code have bad effects more often? If so, what type of UB? Repository: r

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D92270#2422741 , @aqjune wrote: > In D92270#2422661 , @uabelho wrote: > >> Should langref also be updated to describe this? Or does it already? >> I just found this >> "An instruction th

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D92270#2422661 , @uabelho wrote: > Should langref also be updated to describe this? Or does it already? > I just found this > "An instruction that depends on a poison value, produces a poison value > itself." > here > http://l

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. In D92270#2422625 , @aqjune wrote: > Hi, > > It seems it is related to two optimizations: > (1) `select i1 x, true, y` -> `or i1 x, y` > (2) `or i1 x, poison` -> `poison` > > Semantically, the first one is broken. It needs to freez

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Hi, It seems it is related to two optimizations: (1) `select i1 x, true, y` -> `or i1 x, y` (2) `or i1 x, poison` -> `poison` Semantically, the first one is broken. It needs to freeze y. But, it will introduce a lot of freeze instructions. The clang patches that introduc

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-30 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added a comment. Hi, I'm seeing a miscompile with this patch. I'm not very good at the semantic differences between undef and poison so I don't know really where it goes wrong. Before Early CSE I see this in the input: entry: %cmp1 = icmp uge i16 1015, 16, !dbg !9 %shr = lshr

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-29 Thread Juneyoung Lee via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG53040a968dc2: [ConstantFold] Fold more operations to poison (authored by aqjune). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-29 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92270/new/ https://reviews.llvm.org/D92270 ___ cfe

[PATCH] D92270: [ConstantFold] Fold more operations to poison

2020-11-28 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune updated this revision to Diff 308200. aqjune added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. update clang/test/Frontend/fixed_point_unary.c It seems unsigned _Fract type can only represent [0.0, 1.0)? I tried to find a relevant sentence from http:/