public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix ARM pic_add_dot_plus_eight + load peephole (PR target/52006)
@ 2012-01-26 10:47 Jakub Jelinek
  2012-01-26 11:31 ` Ramana Radhakrishnan
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2012-01-26 10:47 UTC (permalink / raw)
  To: Ramana Radhakrishnan; +Cc: gcc-patches

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.

--- 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix ARM pic_add_dot_plus_eight + load peephole (PR target/52006)
  2012-01-26 10:47 [PATCH] Fix ARM pic_add_dot_plus_eight + load peephole (PR target/52006) Jakub Jelinek
@ 2012-01-26 11:31 ` Ramana Radhakrishnan
  0 siblings, 0 replies; 2+ messages in thread
From: Ramana Radhakrishnan @ 2012-01-26 11:31 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2012-01-26 11:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-26 10:47 [PATCH] Fix ARM pic_add_dot_plus_eight + load peephole (PR target/52006) Jakub Jelinek
2012-01-26 11:31 ` Ramana Radhakrishnan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).