Re: [PATCH 3/3] match.pd: build signed low-bit masks in an unsigned type

Andrea Pinski <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <CALvbMcBBJ11vht4niZpVGakqdF+FVUboirjGJ-KscSBGSBx_ag@mail.gmail.com>
On Wed, Aug 19, 2026 at 10:32 PM <[email protected]> wrote:
>
> From: Kyrylo Tkachov <[email protected]>
>
> The PR71636 fold turns
>
>   x & ((1U << b) - 1)
>
> into
>
>   x & ~(~0U << b)
>
> but only when the mask type is unsigned.  Signed source and vector forms keep
> the longer expression.
>
>   int f (int x, int b)
>   {
>     return x & ((1 << b) - 1);
>   }
>
> aarch64 -O2 before:
>
>   f:
>           mov     w2, 1
>           lsl     w2, w2, w1
>           sub     w2, w2, #1
>           and     w0, w2, w0
>           ret
>
> After:
>
>   f:
>           mov     w2, -1
>           lsl     w2, w2, w1
>           bic     w0, w0, w2
>           ret
>
> Build a signed mask in the corresponding unsigned type and convert it back.
> This makes the all-ones shift defined and exposes the shorter form.  The
> preceding vector constant canonicalization lets the vector spelling reach the
> same addition-of-minus-one rule.
>
> The signed form is not valid when the source subtraction can trap or is
> instrumented for overflow.  It can also remove the signed shift-base check for
> the top-bit count.  Keep these cases.  After GIMPLE lowering, an explicit
> shift sanitizer check remains visible, so the fold is safe again.
>
> Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu.
> Ok for trunk?
> Thanks,
> Kyrill
>
> gcc/ChangeLog:
>
>         * match.pd (x & ((1 << b) - 1)): Handle signed scalar and vector
>         types.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/pr71636-signed-1.c: New test.
>         * gcc.dg/tree-ssa/pr71636-signed-vector-1.c: Likewise.
>         * gcc.dg/tree-ssa/pr71636-signed-trap-1.c: Likewise.
>         * gcc.dg/tree-ssa/pr71636-signed-ubsan-1.c: Likewise.
>         * gcc.dg/tree-ssa/pr71636-signed-shift-ubsan-1.c: Likewise.
>
> Signed-off-by: Kyrylo Tkachov <[email protected]>
> ---
>  gcc/match.pd                                  | 14 ++++++++---
>  .../gcc.dg/tree-ssa/pr71636-signed-1.c        | 24 +++++++++++++++++++
>  .../tree-ssa/pr71636-signed-shift-ubsan-1.c   | 11 +++++++++
>  .../gcc.dg/tree-ssa/pr71636-signed-trap-1.c   | 10 ++++++++
>  .../gcc.dg/tree-ssa/pr71636-signed-ubsan-1.c  | 10 ++++++++
>  .../gcc.dg/tree-ssa/pr71636-signed-vector-1.c | 24 +++++++++++++++++++
>  6 files changed, 90 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-shift-ubsan-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-trap-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-ubsan-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-vector-1.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 2c88e56b366..3fb22c12917 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -1577,11 +1577,19 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>      (convert @0)
>      (convert @1)))))
>
> -/* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b);  */
> +/* PR71636: Transform x & ((1U << b) - 1) -> x & ~(~0U << b).  For signed
> +   types, build the mask in the corresponding unsigned type, where shifting
> +   all ones left is defined.  Preserve signed overflow and shift checks.  */
>  (simplify
>    (bit_and:c @0 (plus:s (lshift:s integer_onep @1) integer_minus_onep))
> -  (if (TYPE_UNSIGNED (type))
> -    (bit_and @0 (bit_not (lshift { build_all_ones_cst (type); } @1)))))
> +  (with { tree etype = VECTOR_TYPE_P (type) ? TREE_TYPE (type) : type; }
> +   (if (TYPE_UNSIGNED (etype)
> +       || (!TYPE_OVERFLOW_TRAPS (etype)
> +           && !TYPE_OVERFLOW_SANITIZED (etype)
> +           && (GIMPLE || !sanitize_flags_p (SANITIZE_SHIFT_BASE))))

You don't need to get the element type here. TYPE_UNSIGNED,
TYPE_OVERFLOW_TRAPS and TYPE_OVERFLOW_SANITIZED all work on the
integer vector types as expected. layout_type which is called from
make_vector_type (which is called from build_vector_type) copies
TYPE_SATURATING and TYPE_UNSIGNED from the inner type to the vector
type.

> +    (with { tree utype = unsigned_type_for (type); }
> +     (bit_and @0 (convert
> +                  (bit_not (lshift { build_all_ones_cst (utype); } @1))))))))
>
>  /* PR112533: Canonicalize boolean comparisons of masked pow2 bits into
>     xor-mask tests.
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-1.c
> new file mode 100644
> index 00000000000..9db533fdf64
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-1.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +int
> +f_signed (int x, int b)
> +{
> +  return x & ((1 << b) - 1);
> +}
> +
> +unsigned int
> +f_unsigned (unsigned int x, int b)
> +{
> +  return x & ((1U << b) - 1U);
> +}
> +
> +long
> +f_long (long x, int b)
> +{
> +  return x & ((1L << b) - 1L);
> +}
> +
> +/* { dg-final { scan-tree-dump-not "1 <<" "optimized" } } */
> +/* { dg-final { scan-tree-dump-not " \\+ -1;" "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "= ~" 3 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-shift-ubsan-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-shift-ubsan-1.c
> new file mode 100644
> index 00000000000..3abee74264c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-shift-ubsan-1.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fsanitize=shift-base -fdump-tree-optimized" } */
> +
> +int
> +f (int x, int b)
> +{
> +  return x & ((1 << b) - 1);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "__builtin___ubsan_handle_shift_out_of_bounds" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "= ~" 1 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-trap-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-trap-1.c
> new file mode 100644
> index 00000000000..13a1a5b4cf2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-trap-1.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftrapv -fdump-tree-optimized" } */
> +
> +int
> +f (int x, int b)
> +{
> +  return x & ((1 << b) - 1);
> +}
> +
> +/* { dg-final { scan-tree-dump-times " \\+ -1;" 1 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-ubsan-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-ubsan-1.c
> new file mode 100644
> index 00000000000..68c1b92f18b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-ubsan-1.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fsanitize=signed-integer-overflow -fdump-tree-optimized" } */
> +
> +int
> +f (int x, int b)
> +{
> +  return x & ((1 << b) - 1);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "\\.UBSAN_CHECK_SUB" 1 "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-vector-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-vector-1.c
> new file mode 100644
> index 00000000000..51ba0b35334
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71636-signed-vector-1.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +/* { dg-require-effective-target vect_int } */
> +/* { dg-require-effective-target vect_var_shift } */
> +
> +typedef int v4si __attribute__ ((vector_size (16)));
> +typedef unsigned int v4ui __attribute__ ((vector_size (16)));
> +
> +v4si
> +f_signed (v4si x, v4si b)
> +{
> +  v4si one = { 1, 1, 1, 1 };
> +  return x & ((one << b) - one);
> +}
> +
> +v4ui
> +f_unsigned (v4ui x, v4ui b)
> +{
> +  v4ui one = { 1, 1, 1, 1 };
> +  return x & ((one << b) - one);
> +}
> +
> +/* { dg-final { scan-tree-dump-not "\\{ 1, 1, 1, 1 \\} <<" "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "= ~" 2 "optimized" } } */
> --
> 2.50.1 (Apple Git-155)
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.