diff mbox

Fix ARM pic_add_dot_plus_eight + load peephole (PR target/52006)

Message ID 20120126104711.GH18768@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Jan. 26, 2012, 10:47 a.m. UTC
Hi!

On this testcase we ICE because we have
(insn 65 64 9 2 (set (reg/f:SI 2 r2 [141])
        (unspec:SI [
                (reg/f:SI 2 r2 [141])
                (const_int 8 [0x8])
                (const_int 1 [0x1])
            ] UNSPEC_PIC_BASE)) rh784748.i:13 187 {pic_add_dot_plus_eight}
     (expr_list:REG_EQUAL (symbol_ref:SI ("*.LANCHOR0") [flags 0x182])
        (nil)))

(insn 9 65 10 2 (set (reg:SI 77 s14 [orig:143 b ] [143])
        (mem/c:SI (reg/f:SI 2 r2 [141]) [2 b+0 S4 A32])) rh784748.i:13 640 {*arm_movsi_vfp}
     (expr_list:REG_DEAD (reg/f:SI 2 r2 [141])
        (nil)))
peephole2ed together.  Unfortunately tls_load_pic_dot_plus_eight pattern
uses "=r" constraint for the result, but here we have s14 register instead.
We need to limit this only to mem loads into GENERAL_REGS (or pseudos) in
order to satisfy that constraint.  Peepholes don't have constraints, so
we need to use either some predicate (this fix, apparently that is what
other peephole2 and splitters in arm.md already do) or some condition.

Ok for trunk?

2012-01-26  Jakub Jelinek  <jakub@redhat.com>

	PR target/52006
	* config/arm/arm.md (pic_add_dot_plus_eight peephole2): Use
	arm_general_register_operand predicate for operand 2 instead of
	register_operand.

	* gcc.target/arm/pr52006.c: New test.


	Jakub

Comments

Ramana Radhakrishnan Jan. 26, 2012, 11:31 a.m. UTC | #1
On 26 January 2012 10:47, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On this testcase we ICE because we have
> (insn 65 64 9 2 (set (reg/f:SI 2 r2 [141])
>        (unspec:SI [
>                (reg/f:SI 2 r2 [141])
>                (const_int 8 [0x8])
>                (const_int 1 [0x1])
>            ] UNSPEC_PIC_BASE)) rh784748.i:13 187 {pic_add_dot_plus_eight}
>     (expr_list:REG_EQUAL (symbol_ref:SI ("*.LANCHOR0") [flags 0x182])
>        (nil)))
>
> (insn 9 65 10 2 (set (reg:SI 77 s14 [orig:143 b ] [143])
>        (mem/c:SI (reg/f:SI 2 r2 [141]) [2 b+0 S4 A32])) rh784748.i:13 640 {*arm_movsi_vfp}
>     (expr_list:REG_DEAD (reg/f:SI 2 r2 [141])
>        (nil)))
> peephole2ed together.  Unfortunately tls_load_pic_dot_plus_eight pattern
> uses "=r" constraint for the result, but here we have s14 register instead.
> We need to limit this only to mem loads into GENERAL_REGS (or pseudos) in
> order to satisfy that constraint.  Peepholes don't have constraints, so
> we need to use either some predicate (this fix, apparently that is what
> other peephole2 and splitters in arm.md already do) or some condition.

The analysis is reasonable and it's interesting that we haven't hit this before
even with the softfp variant of the PCS.

Not peephole'ing this form is correct given that the VFP doesn't
really have the [pc, reg]
addressing form.


> Ok for trunk?

OK if no regressions.

Ramana

>
> 2012-01-26  Jakub Jelinek  <jakub@redhat.com>
>
>        PR target/52006
>        * config/arm/arm.md (pic_add_dot_plus_eight peephole2): Use
>        arm_general_register_operand predicate for operand 2 instead of
>        register_operand.
>
>        * gcc.target/arm/pr52006.c: New test.
>
> --- gcc/config/arm/arm.md.jj    2012-01-20 12:35:15.000000000 +0100
> +++ gcc/config/arm/arm.md       2012-01-26 10:24:13.082570508 +0100
> @@ -5719,7 +5719,8 @@ (define_peephole2
>                    (const_int 8)
>                    (match_operand 1 "" "")]
>                   UNSPEC_PIC_BASE))
> -   (set (match_operand:SI 2 "register_operand" "") (mem:SI (match_dup 0)))]
> +   (set (match_operand:SI 2 "arm_general_register_operand" "")
> +       (mem:SI (match_dup 0)))]
>   "TARGET_ARM && peep2_reg_dead_p (2, operands[0])"
>   [(set (match_dup 2)
>        (mem:SI (unspec:SI [(match_dup 3)
> --- gcc/testsuite/gcc.target/arm/pr52006.c.jj   2012-01-26 10:32:27.989658669 +0100
> +++ gcc/testsuite/gcc.target/arm/pr52006.c      2012-01-26 10:32:34.626620068 +0100
> @@ -0,0 +1,19 @@
> +/* PR target/52006 */
> +/* { dg-do compile } */
> +/* { dg-options "-march=armv7-a -mfloat-abi=hard -O2 -fPIC" } */
> +
> +unsigned long a;
> +static int b;
> +
> +void
> +foo (void)
> +{
> +  asm volatile ("" : "=r" (b));
> +}
> +
> +void
> +bar (float f)
> +{
> +  if (f < b / 100.0)
> +    a = 1;
> +}
>
>        Jakub
diff mbox

Patch

--- gcc/config/arm/arm.md.jj	2012-01-20 12:35:15.000000000 +0100
+++ gcc/config/arm/arm.md	2012-01-26 10:24:13.082570508 +0100
@@ -5719,7 +5719,8 @@  (define_peephole2
 		    (const_int 8)
 		    (match_operand 1 "" "")]
 		   UNSPEC_PIC_BASE))
-   (set (match_operand:SI 2 "register_operand" "") (mem:SI (match_dup 0)))]
+   (set (match_operand:SI 2 "arm_general_register_operand" "")
+	(mem:SI (match_dup 0)))]
   "TARGET_ARM && peep2_reg_dead_p (2, operands[0])"
   [(set (match_dup 2)
 	(mem:SI (unspec:SI [(match_dup 3)
--- gcc/testsuite/gcc.target/arm/pr52006.c.jj	2012-01-26 10:32:27.989658669 +0100
+++ gcc/testsuite/gcc.target/arm/pr52006.c	2012-01-26 10:32:34.626620068 +0100
@@ -0,0 +1,19 @@ 
+/* PR target/52006 */
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mfloat-abi=hard -O2 -fPIC" } */
+
+unsigned long a;
+static int b;
+
+void
+foo (void)
+{
+  asm volatile ("" : "=r" (b));
+}
+
+void
+bar (float f)
+{
+  if (f < b / 100.0)
+    a = 1;
+}