[PATCH] i386: Fix ICE during cbranchv16qi4 expansion [PR112681]

[PATCH] i386: Fix ICE during cbranchv16qi4 expansion [PR112681]

[Jakub Jelinek]

Hi!

The following testcase ICEs, because cbranchv16qi4 expansion calls
ix86_expand_branch with op1 being a pre-AVX unaligned memory and
ix86_expand_branch emits a xorv16qi3 instruction without making sure
the operand predicates are satisfied.
While I could manually check if the argument (or both?) doesn't
match vector_operand predicate (apparently this one or bcst_vector_operand
is used in all integral 16+ bytes *xorv*3 instructions) force it into a
register, but as all gen_xorv*3 expanders call
ix86_expand_vector_logical_operator, it seems easier to just call that
function which ensures the right thing happens.  Calling the individual
gen_xorv*3 functions would mean ugly switch on the modes and using high
level expand_simple_binop here seems too high level to me.

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

2023-11-23  Jakub Jelinek  <jakub@redhat.com>

	PR target/112681
	* config/i386/i386-expand.cc (ix86_expand_branch): Use
	ix86_expand_vector_logical_operator to expand vector XOR rather than
	gen_rtx_SET on gen_rtx_XOR.

	* gcc.target/i386/sse4-pr112681.c: New test.

--- gcc/config/i386/i386-expand.cc.jj	2023-11-21 09:31:35.792395304 +0100
+++ gcc/config/i386/i386-expand.cc	2023-11-23 20:57:57.128721762 +0100
@@ -2453,7 +2453,8 @@ ix86_expand_branch (enum rtx_code code,
 	  /* Generate XOR since we can't check that one operand is zero
 	     vector.  */
 	  tmp = gen_reg_rtx (mode);
-	  emit_insn (gen_rtx_SET (tmp, gen_rtx_XOR (mode, op0, op1)));
+	  rtx ops[3] = { tmp, op0, op1 };
+	  ix86_expand_vector_logical_operator (XOR, mode, ops);
 	  tmp = gen_lowpart (p_mode, tmp);
 	  emit_insn (gen_rtx_SET (gen_rtx_REG (CCZmode, FLAGS_REG),
 				  gen_rtx_UNSPEC (CCZmode,
--- gcc/testsuite/gcc.target/i386/sse4-pr112681.c.jj	2023-11-23 21:01:57.983361917 +0100
+++ gcc/testsuite/gcc.target/i386/sse4-pr112681.c	2023-11-23 21:01:42.054584121 +0100
@@ -0,0 +1,11 @@
+/* PR target/112681 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -msse4 -mno-avx" } */
+
+struct S { void *c; char d[16]; } a, b;
+
+int
+foo (void)
+{
+  return __builtin_memcmp (a.d, b.d, sizeof (a.d)) != 0;
+}

	Jakub

Re: [PATCH] i386: Fix ICE during cbranchv16qi4 expansion [PR112681]

[Uros Bizjak]

On Fri, Nov 24, 2023 at 9:31 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following testcase ICEs, because cbranchv16qi4 expansion calls
> ix86_expand_branch with op1 being a pre-AVX unaligned memory and
> ix86_expand_branch emits a xorv16qi3 instruction without making sure
> the operand predicates are satisfied.
> While I could manually check if the argument (or both?) doesn't
> match vector_operand predicate (apparently this one or bcst_vector_operand
> is used in all integral 16+ bytes *xorv*3 instructions) force it into a
> register, but as all gen_xorv*3 expanders call
> ix86_expand_vector_logical_operator, it seems easier to just call that
> function which ensures the right thing happens.  Calling the individual
> gen_xorv*3 functions would mean ugly switch on the modes and using high
> level expand_simple_binop here seems too high level to me.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-11-23  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/112681
>         * config/i386/i386-expand.cc (ix86_expand_branch): Use
>         ix86_expand_vector_logical_operator to expand vector XOR rather than
>         gen_rtx_SET on gen_rtx_XOR.
>
>         * gcc.target/i386/sse4-pr112681.c: New test.

OK.

Thanks,
Uros.

>
> --- gcc/config/i386/i386-expand.cc.jj   2023-11-21 09:31:35.792395304 +0100
> +++ gcc/config/i386/i386-expand.cc      2023-11-23 20:57:57.128721762 +0100
> @@ -2453,7 +2453,8 @@ ix86_expand_branch (enum rtx_code code,
>           /* Generate XOR since we can't check that one operand is zero
>              vector.  */
>           tmp = gen_reg_rtx (mode);
> -         emit_insn (gen_rtx_SET (tmp, gen_rtx_XOR (mode, op0, op1)));
> +         rtx ops[3] = { tmp, op0, op1 };
> +         ix86_expand_vector_logical_operator (XOR, mode, ops);
>           tmp = gen_lowpart (p_mode, tmp);
>           emit_insn (gen_rtx_SET (gen_rtx_REG (CCZmode, FLAGS_REG),
>                                   gen_rtx_UNSPEC (CCZmode,
> --- gcc/testsuite/gcc.target/i386/sse4-pr112681.c.jj    2023-11-23 21:01:57.983361917 +0100
> +++ gcc/testsuite/gcc.target/i386/sse4-pr112681.c       2023-11-23 21:01:42.054584121 +0100
> @@ -0,0 +1,11 @@
> +/* PR target/112681 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -msse4 -mno-avx" } */
> +
> +struct S { void *c; char d[16]; } a, b;
> +
> +int
> +foo (void)
> +{
> +  return __builtin_memcmp (a.d, b.d, sizeof (a.d)) != 0;
> +}
>
>         Jakub
>