vect-patterns: Fix up vect_recog_popcount_clz_ctz_ffs_pattern for non-mode precision types [PR127149]

Message ID apflv8NUnyVenP2s@tucnak
State New
Headers
Series vect-patterns: Fix up vect_recog_popcount_clz_ctz_ffs_pattern for non-mode precision types [PR127149] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap fail Patch failed to apply
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap fail Patch failed to apply

Commit Message

Jakub Jelinek Sept. 2, 2026, 9 a.m. UTC
  Hi!

The following testcase is miscompiled on aarch64-linux.  The problem is that
vect_recog_popcount_clz_ctz_ffs_pattern for builtins other than clz
doesn't require type_has_mode_precision_p (lhs_type) and matches to an ifn
with an argument with say _BitInt(7) type (as well as result) and we then
happily vectorize it and match something that doesn't properly extend the
padding bits.

One possibility is to punt in this case (i.e.
  if (!type_has_mode_precision_p (lhs_type))
    return NULL;
), the following patch instead pattern matches it with a cast to/from
the TREE_TYPE (vec_type), i.e. the actual mode precision type we'll use
for the vectorization.

Bootstrapped/regtested on aarch64-linux, x86_64-linux and i686-linux,
ok for trunk?

2026-09-02  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/127149
	* tree-vect-patterns.cc (vect_recog_popcount_clz_ctz_ffs_pattern):
	Handle lhs_type without mode precision by adding casts.

	* gcc.dg/bitint-142.c: New test.


	Jakub
  

Comments

Richard Biener Sept. 2, 2026, 9:08 a.m. UTC | #1
On Wed, 2 Sep 2026, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled on aarch64-linux.  The problem is that
> vect_recog_popcount_clz_ctz_ffs_pattern for builtins other than clz
> doesn't require type_has_mode_precision_p (lhs_type) and matches to an ifn
> with an argument with say _BitInt(7) type (as well as result) and we then
> happily vectorize it and match something that doesn't properly extend the
> padding bits.
> 
> One possibility is to punt in this case (i.e.
>   if (!type_has_mode_precision_p (lhs_type))
>     return NULL;
> ), the following patch instead pattern matches it with a cast to/from
> the TREE_TYPE (vec_type), i.e. the actual mode precision type we'll use
> for the vectorization.

Do we then vectorize this?  It seems we do not reject conversions
from non-mode-precision types but I also don't see any provisioning
to drop excess bits here or extend.

> Bootstrapped/regtested on aarch64-linux, x86_64-linux and i686-linux,
> ok for trunk?

OK.

Thanks,
Richard.

> 2026-09-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/127149
> 	* tree-vect-patterns.cc (vect_recog_popcount_clz_ctz_ffs_pattern):
> 	Handle lhs_type without mode precision by adding casts.
> 
> 	* gcc.dg/bitint-142.c: New test.
> 
> --- a/gcc/tree-vect-patterns.cc	2026-08-31 17:12:13.972850769 +0200
> +++ b/gcc/tree-vect-patterns.cc	2026-09-01 12:42:48.214480599 +0200
> @@ -2318,6 +2318,17 @@ vect_recog_popcount_clz_ctz_ffs_pattern
>    vect_pattern_detected ("vec_recog_popcount_clz_ctz_ffs_pattern",
>  			 call_stmt);
>  
> +  tree orig_lhs_type = lhs_type;
> +  gimple *cast_stmt = NULL;
> +  if (!type_has_mode_precision_p (lhs_type))
> +    {
> +      lhs_type = TREE_TYPE (vec_type);
> +      cast_stmt
> +	= gimple_build_assign (vect_recog_temp_ssa_var (lhs_type, NULL),
> +			       NOP_EXPR, unprom_diff.op);
> +      unprom_diff.op = gimple_assign_lhs (cast_stmt);
> +    }
> +
>    /* Create B = .POPCOUNT (A).  */
>    new_var = vect_recog_temp_ssa_var (lhs_type, NULL);
>    tree arg2 = NULL_TREE;
> @@ -2358,12 +2369,24 @@ vect_recog_popcount_clz_ctz_ffs_pattern
>  	= vect_recog_ctz_ffs_pattern (vinfo, new_stmt_info, type_out);
>        if (pattern_stmt == NULL)
>  	return NULL;
> +      if (cast_stmt)
> +	append_pattern_def_seq (vinfo, stmt_vinfo, cast_stmt, vec_type);
>        if (gimple_seq seq = STMT_VINFO_PATTERN_DEF_SEQ (new_stmt_info))
>  	{
>  	  gimple_seq *pseq = &STMT_VINFO_PATTERN_DEF_SEQ (stmt_vinfo);
>  	  gimple_seq_add_seq_without_update (pseq, seq);
>  	}
>      }
> +  else if (cast_stmt)
> +    append_pattern_def_seq (vinfo, stmt_vinfo, cast_stmt, vec_type);
> +
> +  if (cast_stmt)
> +    {
> +      append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt, vec_type);
> +      tree ret_var = vect_recog_temp_ssa_var (orig_lhs_type, NULL);
> +      pattern_stmt = gimple_build_assign (ret_var, NOP_EXPR,
> +					  gimple_get_lhs (pattern_stmt));
> +    }
>    return pattern_stmt;
>  }
>  
> --- a/gcc/testsuite/gcc.dg/bitint-142.c	2026-09-01 12:51:40.839450937 +0200
> +++ b/gcc/testsuite/gcc.dg/bitint-142.c	2026-09-01 12:51:21.346708207 +0200
> @@ -0,0 +1,38 @@
> +/* PR tree-optimization/127149 */
> +/* { dg-do run { target bitint } } */
> +/* { dg-options "-O3" } */
> +
> +typedef unsigned _BitInt(7) B;
> +
> +[[gnu::noipa]] void
> +foo (B *restrict y, const B *restrict x)
> +{
> +  for (unsigned i = 0; i < 16; ++i)
> +    y[i] = __builtin_popcountg (x[i]);
> +}
> +
> +[[gnu::noipa]] void
> +bar (B *restrict y, const B *restrict x)
> +{
> +  for (unsigned i = 0; i < 16; ++i)
> +    y[i] = __builtin_ctzg (x[i], 8);
> +}
> +
> +int
> +main ()
> +{
> +  B x[16], y[16], z, w[16];
> +  B e[16] = { 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4 };
> +  B f[16] = { 8, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0 };
> +  for (unsigned i = 0; i < 16; ++i)
> +    x[i] = i;
> +  z = ~0;
> +  __builtin_clear_padding (&z);
> +  for (unsigned i = 0; i < 16; ++i)
> +    ((unsigned char *) &x[0])[i] |= ~*(unsigned char *) &z;
> +  foo (y, x);
> +  bar (w, x);
> +  for (unsigned i = 0; i < 16; ++i)
> +    if (y[i] != e[i] || w[i] != f[i])
> +    __builtin_abort ();
> +}
> 
> 	Jakub
> 
>
  
Jakub Jelinek Sept. 2, 2026, 9:16 a.m. UTC | #2
On Wed, Sep 02, 2026 at 11:08:35AM +0200, Richard Biener wrote:
> > The following testcase is miscompiled on aarch64-linux.  The problem is that
> > vect_recog_popcount_clz_ctz_ffs_pattern for builtins other than clz
> > doesn't require type_has_mode_precision_p (lhs_type) and matches to an ifn
> > with an argument with say _BitInt(7) type (as well as result) and we then
> > happily vectorize it and match something that doesn't properly extend the
> > padding bits.
> > 
> > One possibility is to punt in this case (i.e.
> >   if (!type_has_mode_precision_p (lhs_type))
> >     return NULL;
> > ), the following patch instead pattern matches it with a cast to/from
> > the TREE_TYPE (vec_type), i.e. the actual mode precision type we'll use
> > for the vectorization.
> 
> Do we then vectorize this?

We don't right now.  At least the conversion from the mode precision
type to the non-mode precision one (i.e. on the result of the ifn) seems
to be a deal breaker.  That is why I wrote that just punting is an option
too.  But I think there is no reason why we couldn't add support for
vectorizing those conversions eventually (for _BitInt in particular,
on conversion from _BitInt(7) to signed char or unsigned _BitInt(28)
to unsigned int all one needs is sign or zero extension (which can be
perhaps omitted on targets which guarantee extension in some cases (e.g.
when it is from memory etc.)), and for the other direction on undefined
padding bits targets noop, on others sign or zero extension.

	Jakub
  
Richard Biener Sept. 2, 2026, 9:22 a.m. UTC | #3
On Wed, 2 Sep 2026, Jakub Jelinek wrote:

> On Wed, Sep 02, 2026 at 11:08:35AM +0200, Richard Biener wrote:
> > > The following testcase is miscompiled on aarch64-linux.  The problem is that
> > > vect_recog_popcount_clz_ctz_ffs_pattern for builtins other than clz
> > > doesn't require type_has_mode_precision_p (lhs_type) and matches to an ifn
> > > with an argument with say _BitInt(7) type (as well as result) and we then
> > > happily vectorize it and match something that doesn't properly extend the
> > > padding bits.
> > > 
> > > One possibility is to punt in this case (i.e.
> > >   if (!type_has_mode_precision_p (lhs_type))
> > >     return NULL;
> > > ), the following patch instead pattern matches it with a cast to/from
> > > the TREE_TYPE (vec_type), i.e. the actual mode precision type we'll use
> > > for the vectorization.
> > 
> > Do we then vectorize this?
> 
> We don't right now.  At least the conversion from the mode precision
> type to the non-mode precision one (i.e. on the result of the ifn) seems
> to be a deal breaker.  That is why I wrote that just punting is an option
> too.  But I think there is no reason why we couldn't add support for
> vectorizing those conversions eventually (for _BitInt in particular,
> on conversion from _BitInt(7) to signed char or unsigned _BitInt(28)
> to unsigned int all one needs is sign or zero extension (which can be
> perhaps omitted on targets which guarantee extension in some cases (e.g.
> when it is from memory etc.)), and for the other direction on undefined
> padding bits targets noop, on others sign or zero extension.

Yes, I suppose we could.  I wondered whether there's some
wrong-code lurking .. will try to distill a testcase.

As said, the patch is OK.

Richard.
  
Richard Biener Sept. 2, 2026, 9:26 a.m. UTC | #4
On Wed, 2 Sep 2026, Richard Biener wrote:

> On Wed, 2 Sep 2026, Jakub Jelinek wrote:
> 
> > On Wed, Sep 02, 2026 at 11:08:35AM +0200, Richard Biener wrote:
> > > > The following testcase is miscompiled on aarch64-linux.  The problem is that
> > > > vect_recog_popcount_clz_ctz_ffs_pattern for builtins other than clz
> > > > doesn't require type_has_mode_precision_p (lhs_type) and matches to an ifn
> > > > with an argument with say _BitInt(7) type (as well as result) and we then
> > > > happily vectorize it and match something that doesn't properly extend the
> > > > padding bits.
> > > > 
> > > > One possibility is to punt in this case (i.e.
> > > >   if (!type_has_mode_precision_p (lhs_type))
> > > >     return NULL;
> > > > ), the following patch instead pattern matches it with a cast to/from
> > > > the TREE_TYPE (vec_type), i.e. the actual mode precision type we'll use
> > > > for the vectorization.
> > > 
> > > Do we then vectorize this?
> > 
> > We don't right now.  At least the conversion from the mode precision
> > type to the non-mode precision one (i.e. on the result of the ifn) seems
> > to be a deal breaker.  That is why I wrote that just punting is an option
> > too.  But I think there is no reason why we couldn't add support for
> > vectorizing those conversions eventually (for _BitInt in particular,
> > on conversion from _BitInt(7) to signed char or unsigned _BitInt(28)
> > to unsigned int all one needs is sign or zero extension (which can be
> > perhaps omitted on targets which guarantee extension in some cases (e.g.
> > when it is from memory etc.)), and for the other direction on undefined
> > padding bits targets noop, on others sign or zero extension.
> 
> Yes, I suppose we could.  I wondered whether there's some
> wrong-code lurking .. will try to distill a testcase.

OK, so we handle this with patterns:

struct A { unsigned int a : 27; };

unsigned foo (struct A *a)
{
  unsigned sum = 0;
  for (int i = 0; i < 128; ++i)
    sum += a[i].a;
  return sum;
}

is vectorized as

  vect__ifc__27.7_37 = MEM <vector(4) unsigned int> [(struct A 
*)vectp_a.5_35];
  vect_patt_30.8_38 = vect__ifc__27.7_37 & { 134217727, 134217727, 
134217727, 134217727 };
  vect_sum_10.10_40 = vect_patt_30.8_38 + vect_sum_14.9_39;

via

t.c:6:21: note:   bitfield_ref pattern: detected: _5 = (unsigned int) 
_ifc__28;

and sign-extension is pattern-matched with shifts.

But the BIT_INT type might inhibit those patterns from matching?

> As said, the patch is OK.
> 
> Richard.
> 
>
  

Patch

--- a/gcc/tree-vect-patterns.cc	2026-08-31 17:12:13.972850769 +0200
+++ b/gcc/tree-vect-patterns.cc	2026-09-01 12:42:48.214480599 +0200
@@ -2318,6 +2318,17 @@  vect_recog_popcount_clz_ctz_ffs_pattern
   vect_pattern_detected ("vec_recog_popcount_clz_ctz_ffs_pattern",
 			 call_stmt);
 
+  tree orig_lhs_type = lhs_type;
+  gimple *cast_stmt = NULL;
+  if (!type_has_mode_precision_p (lhs_type))
+    {
+      lhs_type = TREE_TYPE (vec_type);
+      cast_stmt
+	= gimple_build_assign (vect_recog_temp_ssa_var (lhs_type, NULL),
+			       NOP_EXPR, unprom_diff.op);
+      unprom_diff.op = gimple_assign_lhs (cast_stmt);
+    }
+
   /* Create B = .POPCOUNT (A).  */
   new_var = vect_recog_temp_ssa_var (lhs_type, NULL);
   tree arg2 = NULL_TREE;
@@ -2358,12 +2369,24 @@  vect_recog_popcount_clz_ctz_ffs_pattern
 	= vect_recog_ctz_ffs_pattern (vinfo, new_stmt_info, type_out);
       if (pattern_stmt == NULL)
 	return NULL;
+      if (cast_stmt)
+	append_pattern_def_seq (vinfo, stmt_vinfo, cast_stmt, vec_type);
       if (gimple_seq seq = STMT_VINFO_PATTERN_DEF_SEQ (new_stmt_info))
 	{
 	  gimple_seq *pseq = &STMT_VINFO_PATTERN_DEF_SEQ (stmt_vinfo);
 	  gimple_seq_add_seq_without_update (pseq, seq);
 	}
     }
+  else if (cast_stmt)
+    append_pattern_def_seq (vinfo, stmt_vinfo, cast_stmt, vec_type);
+
+  if (cast_stmt)
+    {
+      append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt, vec_type);
+      tree ret_var = vect_recog_temp_ssa_var (orig_lhs_type, NULL);
+      pattern_stmt = gimple_build_assign (ret_var, NOP_EXPR,
+					  gimple_get_lhs (pattern_stmt));
+    }
   return pattern_stmt;
 }
 
--- a/gcc/testsuite/gcc.dg/bitint-142.c	2026-09-01 12:51:40.839450937 +0200
+++ b/gcc/testsuite/gcc.dg/bitint-142.c	2026-09-01 12:51:21.346708207 +0200
@@ -0,0 +1,38 @@ 
+/* PR tree-optimization/127149 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-O3" } */
+
+typedef unsigned _BitInt(7) B;
+
+[[gnu::noipa]] void
+foo (B *restrict y, const B *restrict x)
+{
+  for (unsigned i = 0; i < 16; ++i)
+    y[i] = __builtin_popcountg (x[i]);
+}
+
+[[gnu::noipa]] void
+bar (B *restrict y, const B *restrict x)
+{
+  for (unsigned i = 0; i < 16; ++i)
+    y[i] = __builtin_ctzg (x[i], 8);
+}
+
+int
+main ()
+{
+  B x[16], y[16], z, w[16];
+  B e[16] = { 0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4 };
+  B f[16] = { 8, 0, 1, 0, 2, 0, 1, 0, 3, 0, 1, 0, 2, 0, 1, 0 };
+  for (unsigned i = 0; i < 16; ++i)
+    x[i] = i;
+  z = ~0;
+  __builtin_clear_padding (&z);
+  for (unsigned i = 0; i < 16; ++i)
+    ((unsigned char *) &x[0])[i] |= ~*(unsigned char *) &z;
+  foo (y, x);
+  bar (w, x);
+  for (unsigned i = 0; i < 16; ++i)
+    if (y[i] != e[i] || w[i] != f[i])
+    __builtin_abort ();
+}