[committed,PR,tree-optimization/114277] Fix missed optimization for multiplication against boolean value

Message ID 3a36e210-c305-4cc8-8135-ad91c8c174a0@gmail.com
State New
Headers
Series [committed,PR,tree-optimization/114277] Fix missed optimization for multiplication against boolean value |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-lint warning Lint failed
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
rivoscibot/toolchain-ci-rivos-test success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Jeff Law Feb. 1, 2025, 12:06 a.m. UTC
  Andrew, Raphael and I have all poked at it in various ways over the last 
year or so.  I think when Raphael and I first looked at it I sent us 
down a bit of rathole.

In particular it's odd that we're using a multiply to implement a select 
and it seemed like recognizing the idiom and rewriting into a 
conditional move was the right path.  That looked reasonably good for 
the test, but runs into problems with min/max detection elsewhere.

I think that initial investigation somewhat polluted our thinking.  The 
regression can be fixed with a fairly simple match.pd pattern.

Essentially we want to handle

x * (x || b) -> x
x * !(x || b) -> 0

There's simplifications that can be made for "&&" cases, but I haven't 
seen them in practice.  Rather than drop in untested patterns, I'm 
leaving that as a future todo.

My original was two match.pd patterns.  Andrew combined them into a 
single pattern.  I've made this conditional on GIMPLE as an earlier 
version that simplified to a conditional move showed that when applied 
on GENERIC we could drop an operand with a side effect which is clearly 
not good.

I've bootstrapped and regression tested this on x86.  I've also tested 
on the various embedded targets in my tester.

Pushing to the trunk.

Jeff
commit 2c0a9b7fb7902522fb8484342fcc19fd44df53e6
Author: Jeff Law <jlaw@ventanamicro.com>
Date:   Fri Jan 31 16:59:35 2025 -0700

    [committed][PR tree-optimization/114277] Fix missed optimization for multiplication against boolean value
    
    Andrew, Raphael and I have all poked at it in various ways over the last year
    or so.  I think when Raphael and I first looked at it I sent us down a bit of
    rathole.
    
    In particular it's odd that we're using a multiply to implement a select and it
    seemed like recognizing the idiom and rewriting into a conditional move was the
    right path.  That looked reasonably good for the test, but runs into problems
    with min/max detection elsewhere.
    
    I think that initial investigation somewhat polluted our thinking.  The
    regression can be fixed with a fairly simple match.pd pattern.
    
    Essentially we want to handle
    
    x * (x || b) -> x
    x * !(x || b) -> 0
    
    There's simplifications that can be made for "&&" cases, but I haven't seen
    them in practice.  Rather than drop in untested patterns, I'm leaving that as a
    future todo.
    
    My original was two match.pd patterns.  Andrew combined them into a single
    pattern.  I've made this conditional on GIMPLE as an earlier version that
    simplified to a conditional move showed that when applied on GENERIC we could
    drop an operand with a side effect which is clearly not good.
    
    I've bootstrapped and regression tested this on x86.  I've also tested on the
    various embedded targets in my tester.
    
            PR tree-optimization/114277
    gcc/
            * match.pd (a * (a || b) -> a): New pattern.
            (a * !(a || b) -> 0): Likewise.
    
    gcc/testsuite
            * gcc.target/i386/pr114277.c: New test.
            * gcc.target/riscv/pr114277.c: Likewise.
    
            Co-author:  Andrew Pinski <quic_apinski@quicinc.com>
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 6991868fbe2..97e0bafdda4 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -303,6 +303,23 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (mult @0 integer_zerop@1)
  @1)
 
+#if GIMPLE
+/* When multiplying a value by a boolean involving the value, we may
+   be able to simplify further.
+     a * ((a || b) != 0) -> a
+     a * ((a || b) == 0) -> 0
+
+   There are also bit-and cases which don't show up in practice yet.
+     a * ((a && b) != 0) -> a * b
+     a * ((a && b) == 0) -> b != 0 ? a : b */
+(for neeq (ne eq)
+ (simplify
+  (mult:c (convert? (neeq (bit_ior:c @0 @1) integer_zerop@2)) @0)
+  (if (neeq == EQ_EXPR)
+   { build_zero_cst (type); }
+   @0)))
+#endif
+
 /* -x == x -> x == 0 */
 (for cmp (eq ne)
  (simplify
diff --git a/gcc/testsuite/gcc.target/i386/pr114277.c b/gcc/testsuite/gcc.target/i386/pr114277.c
new file mode 100644
index 00000000000..eb611d26d6a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr114277.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int a,b;
+void func0(int x) { a=x * (x || b); }
+void func1(int x) { a=x * !(x || b); }
+
+/* { dg-final { scan-assembler-not "or" } } */
+/* { dg-final { scan-assembler-not "cmove" } } */
+
diff --git a/gcc/testsuite/gcc.target/riscv/pr114277.c b/gcc/testsuite/gcc.target/riscv/pr114277.c
new file mode 100644
index 00000000000..cc1db19ee4c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr114277.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gc_zicond -mabi=lp64d" { target rv64 } } */
+/* { dg-options "-O2 -march=rv32gc_zicond -mabi=ilp32" { target rv32 } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Os" "-Oz" "-O3" "-Og" } } */
+
+#include "../i386/pr114277.c"
+
+/* { dg-final { scan-assembler-not "czero" } } */
+