On Tue, 31 Mar 2026 at 10:05, Bohan Lei <[email protected]> wrote:
>
> This patch adds missing simplifications for (eq/ne (and/ior x C1) C2) in
> special cases.  In the AND case, when (and C1 C2) is not equal to C1,
> some bits set in C2 are not set in C1, and thus (eq (and x C1) C2) can
> never be true.  The OR case is similar when (and C1 C2) is not equal to
> C2.

Why so narrow? It's easy enough to catch both AND and IOR w/ eq/ne/gtu/leu.
You should be able to optimise at least the following with minimal extra effort:
   (eq (and X 0x880) 1) -> false (the original bug)
   (ne (and X 0x880) 1) -> true
   (gtu (and X 0xff) 0x100) -> false (AND result ≤ 0xff)
   (leu (and X 0xff) 0x100) ->  true
   (eq (ior X 0xf0) 0x01) -> false (result always has 0xf0 set)

> The patch is meant to fix an ICE on RISC-V.  In a former patch, I tried
> to change the insn condition directly, but Jeff pointed out that it was
> more reasonable to optimize it out before the split.  As was suggested
> by Jeff, this patch tries to simplify the expression in
> simplify_relational_operation_1.
>
> The URL for the former patch:
> https://patchwork.sourceware.org/project/gcc/patch/[email protected]/
>
> gcc/ChangeLog:
>
>         * simplify-rtx.cc (simplify_context::simplify_relational_operation_1):
> Add simplifications for (eq/ne (and/ior x C1) C2) in special cases.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/zbs-if_then_else-02.c: New test.
> ---
>  gcc/simplify-rtx.cc                           | 30 +++++++++++++++++
>  .../gcc.target/riscv/zbs-if_then_else-02.c    | 32 +++++++++++++++++++
>  2 files changed, 62 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zbs-if_then_else-02.c
>
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index 50fc51152ca..e2d5abf394f 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -6731,6 +6731,36 @@ simplify_context::simplify_relational_operation_1 
> (rtx_code code,
>         }
>      }
>
> +  /* (eq/ne (and x C1) C2) simplifies to 0/1 if (and C1 C2) is not equal
> +     to C1.  Bits that are zero in C1 will be zero in the result, so if
> +     C1 has any bits set that C2 doesn't have, the comparison can never
> +     be true.  */
> +  if ((code == EQ || code == NE)
> +      && op0code == AND
> +      && CONST_SCALAR_INT_P (op1)
> +      && CONST_SCALAR_INT_P (XEXP (op0, 1)))
> +    {
> +      HOST_WIDE_INT c1 = INTVAL (XEXP (op0, 1));
> +      HOST_WIDE_INT c2 = INTVAL (op1);
> +      if ((c1 & c2) != c1)
> +       return code == EQ ? const0_rtx : const_true_rtx;

That's not correct: your comment says "/* ... if C1 has any bits set
that C2 doesn't have ... */" (and the code implements that), but we
need to check when C2 has bits that C1 doesn't have.
Consider: C1 = 3, C2 = 1 ... (C1 & C2) = 1. You'll see that it doesn't
optimise as expected.

Also, it should be UINTVAL when using bitwise operations.

> +    }
> +
> +  /* (eq/ne (ior x C1) C2) simplifies to 0/1 if (and C1 C2) is not equal
> +     to C2.  Bits that are one in C1 will be one in the result, so if C2
> +     has any bits clear that C1 has set, the comparison can never be
> +     true.  */
> +  if ((code == EQ || code == NE)
> +      && op0code == IOR
> +      && CONST_SCALAR_INT_P (op1)
> +      && CONST_SCALAR_INT_P (XEXP (op0, 1)))
> +    {
> +      HOST_WIDE_INT c1 = INTVAL (XEXP (op0, 1));
> +      HOST_WIDE_INT c2 = INTVAL (op1);
> +      if ((c1 & c2) != c2)
> +       return code == EQ ? const0_rtx : const_true_rtx;
> +    }
> +
>    /* (eq/ne (bswap x) C1) simplifies to (eq/ne x C2) with C2 swapped.  */
>    if ((code == EQ || code == NE)
>        && GET_CODE (op0) == BSWAP
> diff --git a/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-02.c 
> b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-02.c
> new file mode 100644
> index 00000000000..3393bea8898
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/zbs-if_then_else-02.c
> @@ -0,0 +1,32 @@
> +/* { dg-do link  { target { rv64 } } } */
> +/* { dg-options "-march=rv64gc_zbb_zbs -mabi=lp64d -flto" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" } } */
> +
> +struct S {
> +  int a;
> +  char b;
> +  int c;
> +} s;
> +
> +const signed char c = -37;
> +int d;
> +struct S v1[] = {{0, 8}, 0, 0, -108976}, v2[] = {{}, 0, 0, 2804};
> +int a;
> +struct S v3[3];
> +int *p = &a;
> +
> +void foo() {
> +  int a;
> +  if (a)
> +    ;
> +  else if (v1[0].b)
> +    s.a = 0;
> +  else
> +    d = 0;
> +  if (*p)
> +    if (v3[1].c)
> +      if (1 ^ (d & c & v2[1].c & ~v1[1].c | s.a))
> +        v3[2].c = 0;
> +}
> +
> +int main() { foo(); }
> --
> 2.46.0
>

Reply via email to