diff mbox

Fix crossjumping (PR rtl-optimization/58365)

Message ID 20130910103814.GV1817@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Sept. 10, 2013, 10:38 a.m. UTC
Hi!

If two instructions are rtl_equal_p, and cross-jumping decides to cross-jump
them together, merge_memattrs takes care of clearing various MEM_ATTRS
or setting alignment to minimum, size to maximum etc.
But, it didn't handle flags on the MEMs.

The following patch fixes that.  MEM_KEEP_ALIAS_SET_P from quick skimming
seems to be used only during expansion and thus can be ignored IMHO.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8?

BTW, I wonder about REG_ATTRS and REG_POINTER, REG_USERVAR_P,
REG_FUNCTION_VALUE_P, is it fine to keep them as is (as in, can rtx_equal_p
for REG only compares REGNO and mode, nothing else, not sure if we could
end up with differences in any of those during cross-jumping and whether
it could affect code generation).

2013-09-10  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/58365
	* cfgcleanup.c (merge_memattrs): Also clear MEM_READONLY_P
	resp. MEM_NOTRAP_P if they differ, or set MEM_VOLATILE_P if
	it differs.

	* gcc.c-torture/execute/pr58365.c: New test.


	Jakub

Comments

Richard Biener Sept. 10, 2013, 11:18 a.m. UTC | #1
On Tue, Sep 10, 2013 at 12:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> If two instructions are rtl_equal_p, and cross-jumping decides to cross-jump
> them together, merge_memattrs takes care of clearing various MEM_ATTRS
> or setting alignment to minimum, size to maximum etc.
> But, it didn't handle flags on the MEMs.
>
> The following patch fixes that.  MEM_KEEP_ALIAS_SET_P from quick skimming
> seems to be used only during expansion and thus can be ignored IMHO.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8?

Ok.

Thanks,
Richard.

> BTW, I wonder about REG_ATTRS and REG_POINTER, REG_USERVAR_P,
> REG_FUNCTION_VALUE_P, is it fine to keep them as is

Definitely REG_EXPR is used to check for signedness of a register, but I suppose
that is cleared.  I don't see how REG_POINTER can be merged at all, not sure
of the others - they don't seem to have a "conservative correct" value either.

> (as in, can rtx_equal_p
> for REG only compares REGNO and mode, nothing else, not sure if we could
> end up with differences in any of those during cross-jumping and whether
> it could affect code generation).

If REGNO is the same then all of the attributes are, too, no?

Richard.

> 2013-09-10  Jakub Jelinek  <jakub@redhat.com>
>
>         PR rtl-optimization/58365
>         * cfgcleanup.c (merge_memattrs): Also clear MEM_READONLY_P
>         resp. MEM_NOTRAP_P if they differ, or set MEM_VOLATILE_P if
>         it differs.
>
>         * gcc.c-torture/execute/pr58365.c: New test.
>
> --- gcc/cfgcleanup.c.jj 2013-09-09 11:32:42.000000000 +0200
> +++ gcc/cfgcleanup.c    2013-09-10 09:13:43.783165931 +0200
> @@ -925,6 +925,24 @@ merge_memattrs (rtx x, rtx y)
>           set_mem_align (y, MEM_ALIGN (x));
>         }
>      }
> +  if (code == MEM)
> +    {
> +      if (MEM_READONLY_P (x) != MEM_READONLY_P (y))
> +       {
> +         MEM_READONLY_P (x) = 0;
> +         MEM_READONLY_P (y) = 0;
> +       }
> +      if (MEM_NOTRAP_P (x) != MEM_NOTRAP_P (y))
> +       {
> +         MEM_NOTRAP_P (x) = 0;
> +         MEM_NOTRAP_P (y) = 0;
> +       }
> +      if (MEM_VOLATILE_P (x) != MEM_VOLATILE_P (y))
> +       {
> +         MEM_VOLATILE_P (x) = 1;
> +         MEM_VOLATILE_P (y) = 1;
> +       }
> +    }
>
>    fmt = GET_RTX_FORMAT (code);
>    for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
> --- gcc/testsuite/gcc.c-torture/execute/pr58365.c.jj    2013-09-10 09:20:01.761164460 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr58365.c       2013-09-10 09:19:18.000000000 +0200
> @@ -0,0 +1,35 @@
> +/* PR rtl-optimization/58365 */
> +
> +extern void abort (void);
> +
> +struct S
> +{
> +  volatile int a;
> +  int b, c, d, e;
> +} f;
> +static struct S g, h;
> +int i = 1;
> +
> +char
> +foo (void)
> +{
> +  return i;
> +}
> +
> +static struct S
> +bar (void)
> +{
> +  if (foo ())
> +    return f;
> +  return g;
> +}
> +
> +int
> +main ()
> +{
> +  h = bar ();
> +  f.b = 1;
> +  if (h.b != 0)
> +    abort ();
> +  return 0;
> +}
>
>         Jakub
Jakub Jelinek Sept. 10, 2013, 11:27 a.m. UTC | #2
On Tue, Sep 10, 2013 at 01:18:34PM +0200, Richard Biener wrote:
> Ok.

Thanks.

> > BTW, I wonder about REG_ATTRS and REG_POINTER, REG_USERVAR_P,
> > REG_FUNCTION_VALUE_P, is it fine to keep them as is
> 
> Definitely REG_EXPR is used to check for signedness of a register, but I suppose
> that is cleared.  I don't see how REG_POINTER can be merged at all, not sure
> of the others - they don't seem to have a "conservative correct" value either.
> 
> > (as in, can rtx_equal_p
> > for REG only compares REGNO and mode, nothing else, not sure if we could
> > end up with differences in any of those during cross-jumping and whether
> > it could affect code generation).
> 
> If REGNO is the same then all of the attributes are, too, no?

This is after reload, I don't see why the same hard register couldn't be
reused for something else in different part of code, then we cross-jump
those two together.

	Jakub
diff mbox

Patch

--- gcc/cfgcleanup.c.jj	2013-09-09 11:32:42.000000000 +0200
+++ gcc/cfgcleanup.c	2013-09-10 09:13:43.783165931 +0200
@@ -925,6 +925,24 @@  merge_memattrs (rtx x, rtx y)
 	  set_mem_align (y, MEM_ALIGN (x));
 	}
     }
+  if (code == MEM)
+    {
+      if (MEM_READONLY_P (x) != MEM_READONLY_P (y))
+	{
+	  MEM_READONLY_P (x) = 0;
+	  MEM_READONLY_P (y) = 0;
+	}
+      if (MEM_NOTRAP_P (x) != MEM_NOTRAP_P (y))
+	{
+	  MEM_NOTRAP_P (x) = 0;
+	  MEM_NOTRAP_P (y) = 0;
+	}
+      if (MEM_VOLATILE_P (x) != MEM_VOLATILE_P (y))
+	{
+	  MEM_VOLATILE_P (x) = 1;
+	  MEM_VOLATILE_P (y) = 1;
+	}
+    }
 
   fmt = GET_RTX_FORMAT (code);
   for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
--- gcc/testsuite/gcc.c-torture/execute/pr58365.c.jj	2013-09-10 09:20:01.761164460 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr58365.c	2013-09-10 09:19:18.000000000 +0200
@@ -0,0 +1,35 @@ 
+/* PR rtl-optimization/58365 */
+
+extern void abort (void);
+
+struct S
+{
+  volatile int a;
+  int b, c, d, e;
+} f;
+static struct S g, h;
+int i = 1;
+
+char
+foo (void)
+{
+  return i;
+}
+
+static struct S
+bar (void)
+{
+  if (foo ())
+    return f;
+  return g;
+}
+
+int
+main ()
+{
+  h = bar ();
+  f.b = 1;
+  if (h.b != 0)
+    abort ();
+  return 0;
+}