aarch64: Fix aarch64_expand_subvti constant handling [PR93335]
diff mbox series

Message ID 20200122084731.GD10088@tucnak
State New
Headers show
Series
  • aarch64: Fix aarch64_expand_subvti constant handling [PR93335]
Related show

Commit Message

Jakub Jelinek Jan. 22, 2020, 8:47 a.m. UTC
Hi!

The two patterns that call aarch64_expand_subvti ensure that {low,high}_in1
is a register, while {low,high}_in2 can be a register or immediate.
subdi3_compare1_imm uses the aarch64_plus_immediate predicate for its last
two operands (the value and negated value), but aarch64_expand_subvti calls
it whenever low_in2 is a CONST_INT, which leads to ICEs during vregs pass,
as the emitted insn is not recognized as valid subdi3_compare1_imm.
The following patch fixes that by only using subdi3_compare1_imm if it is ok
to do so, and otherwise force the constant into register and use the
non-immediate version - subdi3_compare1.
Furthermore, previously the code was calling force_reg on high_in2 only if
low_in2 is CONST_INT, on the (reasonable) assumption is that only if low_in2
is a CONST_INT, high_in2 can be non-REG, but with the above changes even in
the else we might have CONST_INT and force_reg doesn't do anything if the
operand is already a REG, so this patch calls it unconditionally.

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

2020-01-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/93335
	* config/aarch64/aarch64.c (aarch64_expand_subvti): Only use
	gen_subdi3_compare1_imm if low_in2 satisfies aarch64_plus_immediate
	predicate, not whenever it is CONST_INT.  Otherwise, force_reg it.
	Call force_reg on high_in2 unconditionally.

	* gcc.c-torture/compile/pr93335.c: New test.


	Jakub

Comments

Richard Sandiford Jan. 22, 2020, 10:56 a.m. UTC | #1
Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> The two patterns that call aarch64_expand_subvti ensure that {low,high}_in1
> is a register, while {low,high}_in2 can be a register or immediate.
> subdi3_compare1_imm uses the aarch64_plus_immediate predicate for its last
> two operands (the value and negated value), but aarch64_expand_subvti calls
> it whenever low_in2 is a CONST_INT, which leads to ICEs during vregs pass,
> as the emitted insn is not recognized as valid subdi3_compare1_imm.
> The following patch fixes that by only using subdi3_compare1_imm if it is ok
> to do so, and otherwise force the constant into register and use the
> non-immediate version - subdi3_compare1.
> Furthermore, previously the code was calling force_reg on high_in2 only if
> low_in2 is CONST_INT, on the (reasonable) assumption is that only if low_in2
> is a CONST_INT, high_in2 can be non-REG, but with the above changes even in
> the else we might have CONST_INT and force_reg doesn't do anything if the
> operand is already a REG, so this patch calls it unconditionally.
>
> Bootstrapped/regtested on aarch64-linux, ok for trunk and 9.3?
>
> 2020-01-22  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR target/93335
> 	* config/aarch64/aarch64.c (aarch64_expand_subvti): Only use
> 	gen_subdi3_compare1_imm if low_in2 satisfies aarch64_plus_immediate
> 	predicate, not whenever it is CONST_INT.  Otherwise, force_reg it.
> 	Call force_reg on high_in2 unconditionally.
>
> 	* gcc.c-torture/compile/pr93335.c: New test.

OK, thanks.

Richard

> --- gcc/config/aarch64/aarch64.c.jj	2020-01-21 17:52:54.932504456 +0100
> +++ gcc/config/aarch64/aarch64.c	2020-01-21 20:04:09.992921849 +0100
> @@ -20193,14 +20193,15 @@ aarch64_expand_subvti (rtx op0, rtx low_
>      }
>    else
>      {
> -      if (CONST_INT_P (low_in2))
> +      if (aarch64_plus_immediate (low_in2, DImode))
> +	emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
> +					    GEN_INT (-INTVAL (low_in2))));
> +      else
>  	{
> -	  high_in2 = force_reg (DImode, high_in2);
> -	  emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
> -					      GEN_INT (-INTVAL (low_in2))));
> +	  low_in2 = force_reg (DImode, low_in2);
> +	  emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
>  	}
> -      else
> -	emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
> +      high_in2 = force_reg (DImode, high_in2);
>  
>        if (unsigned_p)
>  	emit_insn (gen_usubdi3_carryinC (high_dest, high_in1, high_in2));
> --- gcc/testsuite/gcc.c-torture/compile/pr93335.c.jj	2020-01-21 20:04:27.728654103 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr93335.c	2020-01-21 20:03:55.384142397 +0100
> @@ -0,0 +1,98 @@
> +/* PR target/93335 */
> +/* { dg-do compile { target int128 } } */
> +
> +int
> +f1 (unsigned int x)
> +{
> +  return __builtin_sub_overflow_p (x, 4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f2 (unsigned int x)
> +{
> +  return __builtin_sub_overflow_p (x, 4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f3 (int x)
> +{
> +  return __builtin_sub_overflow_p (x, 4096, (__int128) 0);
> +}
> +
> +int
> +f4 (int x)
> +{
> +  return __builtin_sub_overflow_p (x, 4097, (__int128) 0);
> +}
> +
> +int
> +f5 (unsigned int x)
> +{
> +  return __builtin_sub_overflow_p (x, -4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f6 (unsigned int x)
> +{
> +  return __builtin_sub_overflow_p (x, -4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f7 (int x)
> +{
> +  return __builtin_sub_overflow_p (x, -4096, (__int128) 0);
> +}
> +
> +int
> +f8 (int x)
> +{
> +  return __builtin_sub_overflow_p (x, -4097, (__int128) 0);
> +}
> +
> +int
> +f9 (unsigned int x)
> +{
> +  return __builtin_add_overflow_p (x, 4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f10 (unsigned int x)
> +{
> +  return __builtin_add_overflow_p (x, 4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f11 (int x)
> +{
> +  return __builtin_add_overflow_p (x, 4096, (__int128) 0);
> +}
> +
> +int
> +f12 (int x)
> +{
> +  return __builtin_add_overflow_p (x, 4097, (__int128) 0);
> +}
> +
> +int
> +f13 (unsigned int x)
> +{
> +  return __builtin_add_overflow_p (x, -4096, (unsigned __int128) 0);
> +}
> +
> +int
> +f14 (unsigned int x)
> +{
> +  return __builtin_add_overflow_p (x, -4097, (unsigned __int128) 0);
> +}
> +
> +int
> +f15 (int x)
> +{
> +  return __builtin_add_overflow_p (x, -4096, (__int128) 0);
> +}
> +
> +int
> +f16 (int x)
> +{
> +  return __builtin_add_overflow_p (x, -4097, (__int128) 0);
> +}
>
> 	Jakub

Patch
diff mbox series

--- gcc/config/aarch64/aarch64.c.jj	2020-01-21 17:52:54.932504456 +0100
+++ gcc/config/aarch64/aarch64.c	2020-01-21 20:04:09.992921849 +0100
@@ -20193,14 +20193,15 @@  aarch64_expand_subvti (rtx op0, rtx low_
     }
   else
     {
-      if (CONST_INT_P (low_in2))
+      if (aarch64_plus_immediate (low_in2, DImode))
+	emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
+					    GEN_INT (-INTVAL (low_in2))));
+      else
 	{
-	  high_in2 = force_reg (DImode, high_in2);
-	  emit_insn (gen_subdi3_compare1_imm (low_dest, low_in1, low_in2,
-					      GEN_INT (-INTVAL (low_in2))));
+	  low_in2 = force_reg (DImode, low_in2);
+	  emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
 	}
-      else
-	emit_insn (gen_subdi3_compare1 (low_dest, low_in1, low_in2));
+      high_in2 = force_reg (DImode, high_in2);
 
       if (unsigned_p)
 	emit_insn (gen_usubdi3_carryinC (high_dest, high_in1, high_in2));
--- gcc/testsuite/gcc.c-torture/compile/pr93335.c.jj	2020-01-21 20:04:27.728654103 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr93335.c	2020-01-21 20:03:55.384142397 +0100
@@ -0,0 +1,98 @@ 
+/* PR target/93335 */
+/* { dg-do compile { target int128 } } */
+
+int
+f1 (unsigned int x)
+{
+  return __builtin_sub_overflow_p (x, 4096, (unsigned __int128) 0);
+}
+
+int
+f2 (unsigned int x)
+{
+  return __builtin_sub_overflow_p (x, 4097, (unsigned __int128) 0);
+}
+
+int
+f3 (int x)
+{
+  return __builtin_sub_overflow_p (x, 4096, (__int128) 0);
+}
+
+int
+f4 (int x)
+{
+  return __builtin_sub_overflow_p (x, 4097, (__int128) 0);
+}
+
+int
+f5 (unsigned int x)
+{
+  return __builtin_sub_overflow_p (x, -4096, (unsigned __int128) 0);
+}
+
+int
+f6 (unsigned int x)
+{
+  return __builtin_sub_overflow_p (x, -4097, (unsigned __int128) 0);
+}
+
+int
+f7 (int x)
+{
+  return __builtin_sub_overflow_p (x, -4096, (__int128) 0);
+}
+
+int
+f8 (int x)
+{
+  return __builtin_sub_overflow_p (x, -4097, (__int128) 0);
+}
+
+int
+f9 (unsigned int x)
+{
+  return __builtin_add_overflow_p (x, 4096, (unsigned __int128) 0);
+}
+
+int
+f10 (unsigned int x)
+{
+  return __builtin_add_overflow_p (x, 4097, (unsigned __int128) 0);
+}
+
+int
+f11 (int x)
+{
+  return __builtin_add_overflow_p (x, 4096, (__int128) 0);
+}
+
+int
+f12 (int x)
+{
+  return __builtin_add_overflow_p (x, 4097, (__int128) 0);
+}
+
+int
+f13 (unsigned int x)
+{
+  return __builtin_add_overflow_p (x, -4096, (unsigned __int128) 0);
+}
+
+int
+f14 (unsigned int x)
+{
+  return __builtin_add_overflow_p (x, -4097, (unsigned __int128) 0);
+}
+
+int
+f15 (int x)
+{
+  return __builtin_add_overflow_p (x, -4096, (__int128) 0);
+}
+
+int
+f16 (int x)
+{
+  return __builtin_add_overflow_p (x, -4097, (__int128) 0);
+}