diff mbox

Fix up get_reload_reg (PR rtl-optimization/56195)

Message ID 20130208002702.GL4385@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 8, 2013, 12:27 a.m. UTC
Hi!

As the following testcase shows, get_reload_reg could sometimes return
a reused reg in a wrong mode.  If curr_insn_input_reloads[i].input
and original have VOIDmode, they could compare equal (same integer
constant), yet mode might be different from
GET_MODE (curr_insn_input_reloads[i].reg).
If we need a wider reg than we already have, we can't reuse it easily,
but if we need a narrower one, we can just use a subreg of the wider one.

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

2013-02-07  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/56195
	* lra-constraints.c (get_reload_reg): Don't reuse regs
	if they have smaller mode than requested, if they have
	wider mode than requested, try to return a SUBREG.

	* gcc.dg/torture/pr56195.c: New test.


	Jakub

Comments

David Miller Feb. 8, 2013, 2:01 a.m. UTC | #1
From: Jakub Jelinek <jakub@redhat.com>
Date: Fri, 8 Feb 2013 01:27:02 +0100

> +void
> +fn (void)
> +{
> +  if (b)
> +    {
> +      int *p, *q;
> +      char g;
> +
> +      if (f++)
> +	for (;; e++);
> +    lbl:
> +      for (b = 0; b < 2; b++)
> +	t /= d + 1 ? : i || a < c < (d = f) ? : 1 | (j = 2);
> +
> +      *p = g >= *q ^ c != a ^ *p;

I know this is "just a testcase" but it looks like both 'p' and 'q'
can be uninitialized at this point.
Jakub Jelinek Feb. 8, 2013, 8:08 a.m. UTC | #2
On Thu, Feb 07, 2013 at 09:01:15PM -0500, David Miller wrote:
> From: Jakub Jelinek <jakub@redhat.com>
> Date: Fri, 8 Feb 2013 01:27:02 +0100
> 
> > +void
> > +fn (void)
> > +{
> > +  if (b)
> > +    {
> > +      int *p, *q;
> > +      char g;
> > +
> > +      if (f++)
> > +	for (;; e++);
> > +    lbl:
> > +      for (b = 0; b < 2; b++)
> > +	t /= d + 1 ? : i || a < c < (d = f) ? : 1 | (j = 2);
> > +
> > +      *p = g >= *q ^ c != a ^ *p;
> 
> I know this is "just a testcase" but it looks like both 'p' and 'q'
> can be uninitialized at this point.

Yeah, that is why it is just an dg-do assemble testcase, not dg-do run.
The uninitialized vars are necessary to reproduce the problem, at least
I haven't managed to adjust the testcase to reproduce without those.

	Jakub
Vladimir Makarov Feb. 8, 2013, 3:12 p.m. UTC | #3
On 13-02-07 7:27 PM, Jakub Jelinek wrote:
> Hi!
>
> As the following testcase shows, get_reload_reg could sometimes return
> a reused reg in a wrong mode.  If curr_insn_input_reloads[i].input
> and original have VOIDmode, they could compare equal (same integer
> constant), yet mode might be different from
> GET_MODE (curr_insn_input_reloads[i].reg).
> If we need a wider reg than we already have, we can't reuse it easily,
> but if we need a narrower one, we can just use a subreg of the wider one.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2013-02-07  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/56195
> 	* lra-constraints.c (get_reload_reg): Don't reuse regs
> 	if they have smaller mode than requested, if they have
> 	wider mode than requested, try to return a SUBREG.
>
> 	* gcc.dg/torture/pr56195.c: New test.
>
Sure, Jakub.  Thanks.
diff mbox

Patch

--- gcc/lra-constraints.c.jj	2013-02-07 18:34:39.000000000 +0100
+++ gcc/lra-constraints.c	2013-02-07 20:58:25.558920536 +0100
@@ -421,8 +421,20 @@  get_reload_reg (enum op_type type, enum
       if (rtx_equal_p (curr_insn_input_reloads[i].input, original)
 	  && in_class_p (curr_insn_input_reloads[i].reg, rclass, &new_class))
 	{
-	  *result_reg = curr_insn_input_reloads[i].reg;
-	  regno = REGNO (*result_reg);
+	  rtx reg = curr_insn_input_reloads[i].reg;
+	  regno = REGNO (reg);
+	  /* If input is equal to original and both are VOIDmode,
+	     GET_MODE (reg) might be still different from mode.
+	     Ensure we don't return *result_reg with wrong mode.  */
+	  if (GET_MODE (reg) != mode)
+	    {
+	      if (GET_MODE_SIZE (GET_MODE (reg)) < GET_MODE_SIZE (mode))
+		continue;
+	      reg = lowpart_subreg (mode, reg, GET_MODE (reg));
+	      if (reg == NULL_RTX || GET_CODE (reg) != SUBREG)
+		continue;
+	    }
+	  *result_reg = reg;
 	  if (lra_dump_file != NULL)
 	    {
 	      fprintf (lra_dump_file, "	 Reuse r%d for reload ", regno);
--- gcc/testsuite/gcc.dg/torture/pr56195.c.jj	2013-02-07 21:08:34.244334032 +0100
+++ gcc/testsuite/gcc.dg/torture/pr56195.c	2013-02-07 21:08:18.000000000 +0100
@@ -0,0 +1,31 @@ 
+/* PR rtl-optimization/56195 */
+/* { dg-do assemble } */
+
+int i, a, t, b, c, d, e, f, j, *h;
+
+void
+fn (void)
+{
+  if (b)
+    {
+      int *p, *q;
+      char g;
+
+      if (f++)
+	for (;; e++);
+    lbl:
+      for (b = 0; b < 2; b++)
+	t /= d + 1 ? : i || a < c < (d = f) ? : 1 | (j = 2);
+
+      *p = g >= *q ^ c != a ^ *p;
+
+      if (!e)
+	{
+	  q = p;
+	  goto lbl;
+	}
+    }
+
+  if (h++)
+    goto lbl;
+}