[PATCH] Fix ia64 .copy_state/.body directive emission (PR target/32338, take 2)

Jakub Jelinek jakub@redhat.com
Wed Sep 12 18:01:00 GMT 2007


On Tue, Sep 11, 2007 at 03:21:51PM -0700, Jim Wilson wrote:
> I think the only simple solution is to emit the blockage insn.  I don't
> think this problem is specific to sibcalls, so I think we need the Q1
> patch attachment in pr 32338.  This patch looks OK to me.

Ok, I have bootstrapped/regtested the following patch.

> Another possible solution here is to figure out why we have the
> redundant sp restore instructions.  If we could optimize away the
> duplicate, then the immediate problem disappears, as now we only need
> one restore sp directive.  However, I think the general problem still
> exists, in that the sp restore instruction isn't necessarily in the
> block immediately before the sibcall/return, and hence we still need the
> blockage instruction to be safe.

The scheduler moves the sp restore because it sees no harm in doing so,
has free slot in some bundle in an earlier bb within ebb, sp isn't
used anywhere between the new bundle slot and the location in both bb's where it
has been previously and the reg from which we are restoring it isn't either.
But as this is the last pass, there is no DCE afterwards and the scheduler
doesn't do DCE on its own.  But even if the scheduler eliminated the second,
after the move redundant restore, would unwind info be valid?

BTW, could you please also review PR32337 fix?  I have
bootstrapped/regtested that fix today as well.

Thanks.

	Jakub
-------------- next part --------------
2007-09-12  Jakub Jelinek  <jakub@redhat.com>

	PR target/32338
	* config/ia64/ia64.c (ia64_expand_epilogue): Emit blockage
	before sp restoration even when total_size is 0, but
	frame_pointer_needed.

	* gcc.dg/pr32338-1.c: New test.
	* gcc.dg/pr32338-2.c: New test.

--- gcc/config/ia64/ia64.c.jj	2007-07-12 13:59:23.000000000 +0200
+++ gcc/config/ia64/ia64.c	2007-07-16 18:45:02.000000000 +0200
@@ -3434,7 +3434,9 @@ ia64_expand_epilogue (int sibcall_p)
 
   finish_spill_pointers ();
 
-  if (current_frame_info.total_size || cfun->machine->ia64_eh_epilogue_sp)
+  if (current_frame_info.total_size
+      || cfun->machine->ia64_eh_epilogue_sp
+      || frame_pointer_needed)
     {
       /* ??? At this point we must generate a magic insn that appears to
          modify the spill iterators, the stack pointer, and the frame
--- gcc/testsuite/gcc.dg/pr32338-1.c.jj	2007-09-05 08:39:37.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr32338-1.c	2007-09-05 08:06:29.000000000 +0200
@@ -0,0 +1,47 @@
+/* PR target/32338 */
+/* { dg-do link } */
+/* { dg-options "-O2 -fno-omit-frame-pointer" } */
+
+struct S
+{
+};
+
+int
+__attribute__((noinline))
+foo (struct S *d)
+{
+  return 2;
+}
+
+int
+__attribute__((noinline))
+bar (struct S *d)
+{
+  return 4;
+}
+
+int
+__attribute__((noinline))
+fnl (char const *q)
+{
+  return __builtin_strlen (q);
+}
+
+int
+__attribute__((noinline))
+baz (struct S *d, char const *q)
+{
+  unsigned int len;
+  len = fnl (q);
+  if (len > 512)
+    return bar (d);
+  return foo (d);
+}
+
+int
+main (int argc, char *argv[])
+{
+  if (argc > 30)
+    return baz ((void *) 0, "abcde");
+  return 0;
+}
--- gcc/testsuite/gcc.dg/pr32338-2.c.jj	2007-09-05 08:39:43.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr32338-2.c	2007-09-05 08:28:25.000000000 +0200
@@ -0,0 +1,47 @@
+/* PR target/32338 */
+/* { dg-do link } */
+/* { dg-options "-O2 -fno-omit-frame-pointer" } */
+
+struct S
+{
+};
+
+int
+__attribute__((noinline))
+foo (void)
+{
+  return 2;
+}
+
+int
+__attribute__((noinline))
+bar (void)
+{
+  return 4;
+}
+
+int
+__attribute__((noinline))
+fnl (void)
+{
+  return 6;
+}
+
+int
+__attribute__((noinline))
+baz (void)
+{
+  unsigned int len;
+  len = fnl ();
+  if (len > 512)
+    return bar ();
+  return foo ();
+}
+
+int
+main (int argc, char *argv[])
+{
+  if (argc > 30)
+    return baz ();
+  return 0;
+}


More information about the Gcc-patches mailing list