[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. In D101191#2788596 , @spatel wrote: > In D101191#2783570 , @rupprecht > wrote: > >> In D101191#2782963 , @rupprecht >> wrote: >> >>> The

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added a comment. In D101191#2783570 , @rupprecht wrote: > In D101191#2782963 , @rupprecht > wrote: > >> The issue I'm seeing seems more directly caused by SLP vectorization, as it >> goes away with

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. In D101191#2782963 , @rupprecht wrote: > The issue I'm seeing seems more directly caused by SLP vectorization, as it > goes away with `-fno-slp-vectorize`. This patch merely unblocks that bad > optimization AFAICT. Filed as

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. The issue I'm seeing seems more directly caused by SLP vectorization, as it goes away with `-fno-slp-vectorize`. This patch merely unblocks that bad optimization AFAICT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

rupprecht added a comment. FYI, I'm seeing what I think is a miscompile that bisects to this patch. Greatly simplified, the problematic snippet is this: struct Stats { int a; int b; int a_or_b; }; bool x = ... bool y = ... Stats stats; stats.a = x ? 1 : 0; stats.b =

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added a comment. In D101191#2779631 , @spatel wrote: > In D101191#2779007 , @fhahn wrote: > >> Looks like this is causing an infinite loop in instcombine: >>

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added a comment. In D101191#2779007 , @fhahn wrote: > Looks like this is causing an infinite loop in instcombine: > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34661 This seems likely to be another partial undef/poison vector problem (

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

fhahn added a comment. Herald added a subscriber: foad. Looks like this is causing an infinite loop in instcombine: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34661 Reproducer test case: https://oss-fuzz.com/download?testcase_id=6383789942112256 , hangs with `opt -instcombine`,

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added a comment. In D101191#2751002 , @sanwou01 wrote: > Hi, we've got a ~6% regression in SPEC INT 2006 462.libquantum on AArch64 > (both -flto and -Ofast) that comes back to this change. See here for a > reproducer

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

sanwou01 added subscribers: dmgreen, sanwou01. sanwou01 added a comment. Hi, we've got a ~6% regression in SPEC INT 2006 462.libquantum on AArch64 (both -flto and -Ofast) that comes back to this change. See here for a reproducer https://godbolt.org/z/dq98Gqqxn (-fno-vectorize is not strictly

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. In D101191#2738130 , @nikic wrote: > LGTM. I can take care of reverting if there are issues. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM. I can take care of reverting if there are issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. I think this patch is ready to go: running the test-suite on an ARM machine didn't complain anything. Well, but one thing that I'm concerned about is that from tomorrow I'll not be online for about three weeks. :( I'd like to find someone who reverts this patch if

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/or.ll:1135 ; CHECK-NEXT:ret i1 [[OR]] ; %x = icmp sge i16 %a, %b This can be salvaged as well: https://alive2.llvm.org/ce/z/yXF96T But I think there are more patterns that are

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/or.ll:1102 +; CHECK-NEXT:[[AND:%.*]] = select i1 [[Y]], i1 [[X]], i1 false +; CHECK-NEXT:[[OR:%.*]] = select i1 [[X_INV]], i1 true, i1 [[AND]] ; CHECK-NEXT:ret i1 [[OR]]

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added a comment. Went through all the tests again, looks like there's three patterns that could still be handled. I think the last one of them should be handled, and for the other two it's fine to just open an issue. Comment at:

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. (BTW, if the patch is reviewed, then I believe we are finally ready to land this patch.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101191/new/ https://reviews.llvm.org/D101191

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. I made D101720 to cover the remaining cases except `Transforms/InstCombine/and2.ll`. Supporting `and2.ll` doesn't seem super-straightforward, but maybe some minor tweaks can make it. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added a comment. I found that there are a few more patterns that can be salvaged. Will create a patch for them. Comment at: llvm/test/Transforms/InstCombine/and.ll:898 ; CHECK-NEXT:[[Y:%.*]] = icmp ugt i72 [[C:%.*]], 42 -; CHECK-NEXT:[[AND:%.*]] = and i1

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { aqjune wrote: > nikic wrote: > > spatel

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { nikic wrote: > spatel wrote: > > aqjune

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { spatel wrote: > aqjune wrote: > > aqjune

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { aqjune wrote: > aqjune wrote: > > spatel

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { aqjune wrote: > spatel wrote: > > Any ideas

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; nikic wrote: > aqjune wrote: > > nikic wrote: > > > It

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/unsigned-multiply-overflow-check.ll:20 +; FIXME: noundef should be attached to args define i1 @will_not_overflow(i64 %arg, i64 %arg1) { Any ideas about what it will take to get

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

spatel added inline comments. Comment at: llvm/test/Transforms/PhaseOrdering/X86/vector-reductions.ll:282 +; FIXME: this should be vectorized define i1 @cmp_lt_gt(double %a, double %b, double %c) { I don't think we need to worry about regressing this. It's

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; aqjune wrote: > nikic wrote: > > It looks like this fold

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2680 +// select c, (select a, true, b), false -> select c, a, false +// if c implies that b is false. +if (match(CondVal, m_Select(m_Value(A), m_One(), m_Value(B))) &&

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

aqjune added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; nikic wrote: > It looks like this fold could be salvaged,

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

nikic added inline comments. Comment at: llvm/test/Transforms/InstCombine/logical-select.ll:385 +; CHECK-NEXT:[[OR:%.*]] = select i1 [[AND1]], i1 true, i1 [[AND2]] +; CHECK-NEXT:ret i1 [[OR]] ; It looks like this fold could be salvaged, if we wanted to: