Patch to gcc/resource.c for PR target/13256, CRIS, SH, reorg, dbr, strict_low_part.

Hans-Peter Nilsson hans-peter.nilsson@axis.com
Mon Dec 1 06:20:00 GMT 2003


It seems the CRIS and SH ports are the only ones describing delay-slots
and mentioning strict_low_part in the port, necessary to exercise this
bug.  I'm a bit distressed finding that mark_set_resources (and
resource.c) is only used by the delayed-branch pass.

Referring to the patch, perhaps it would have been enough to just change
it to pass in_dest in the recursion (the second changed line).  However, I
couldn't find the logic for the mark_type test here; the in_dest test
should cover its need.  It looks like a thinko.  This STRICT_LOW_PART case
was introduced with resource.c CVS rev. 1.28 and was aired on gcc-patches
archived as <URL:http://gcc.gnu.org/ml/gcc-patches/2000-03/msg00914.html>,
but was not in an earlier version and is not referred in the ChangeLog
entry.  From that thread, it is also not apparent why this case was
introduced.  CC to Stan Cox, in hope that he can shed some light.  I hope
Stan's address is valid (taken from the latest email referring to him by
name); he's not mentioned in MAINTAINERS (hint, hint).  It'd be
interesting to hear how this code-path was tested, because it doesn't seem
to be exercised by x86 or sparc.

As the code alludes, STRICT_LOW_PART is now handled similar to SUBREG,
because that's what it is, a SUBREG with defined semantics: when in
destination, parts outside the STRICT_LOW_PART aren't touched, when in
source, they aren't referred.  An alternative solution would be to remove
the case and let the generic code handle this, but I think it's better to
keep it specific and visible, to avoid introducing a similar thinko before
resource.c goes away in preference for other data dependency tracking (in
a presumed reorg.c rewrite).  A third solution would be to remove all
strict_low_part from GCC, as mentioned in the past.

The patch is tested cross to mips-elf+mips-sim sh-elf+sh-sim and
cris-axis-elf with simulator, no Ada, no regressions, fixes the
test-case for cris-axis-elf.  It also fixes
gcc.c-torture/execute/990628-1.c for cris-axis-elf which has
been failing for the same reason.

Ok to commit?
Gaby, is ok for 3.3-branch too (given same testing there)?

gcc:
	PR target/13256
	* resource.c (mark_set_resources) <case STRICT_LOW_PART>: Recurse
	if in_dest, and pass in_dest.

gcc/testsuite:
	PR target/13256
	* gcc.c-torture/execute/20031201-1.c: New test.

--- /dev/null	Tue Jan  1 05:00:00 1980
+++ gcc.c-torture/execute/20031201-1.c	Mon Dec  1 05:37:00 2003
@@ -0,0 +1,76 @@
+/* Copyright (C) 2003  Free Software Foundation.
+   PR target/13256
+   STRICT_LOW_PART was handled incorrectly in delay slots.
+   Origin: Hans-Peter Nilsson.  */
+
+typedef struct { unsigned int e0 : 16; unsigned int e1 : 16; } s1;
+typedef struct { unsigned int e0 : 16; unsigned int e1 : 16; } s2;
+typedef struct { s1 i12; s2 i16; } io;
+static int test_length = 2;
+static io *i;
+static int m = 1;
+static int d = 1;
+static unsigned long test_t0;
+static unsigned long test_t1;
+void test(void) __attribute__ ((__noinline__));
+extern int f1 (void *port) __attribute__ ((__noinline__));
+extern void f0 (void) __attribute__ ((__noinline__));
+int
+f1 (void *port)
+{
+  int fail_count = 0;
+  unsigned long tlen;
+  s1 x0 = {0};
+  s2 x1 = {0};
+
+  i = port;
+  x0.e0 = x1.e0 = 32;
+  i->i12 = x0;
+  i->i16 = x1;
+  do f0(); while (test_t1);
+  x0.e0 = x1.e0 = 8;
+  i->i12 = x0;
+  i->i16 = x1;
+  test ();
+  if (m)
+    {
+      unsigned long e = 1000000000 / 460800 * test_length;
+      tlen = test_t1 - test_t0;
+      if (((tlen-e) & 0x7FFFFFFF) > 1000)
+	f0();
+    }
+  if (d)
+    {
+      unsigned long e = 1000000000 / 460800 * test_length;
+      tlen = test_t1 - test_t0;
+      if (((tlen - e) & 0x7FFFFFFF) > 1000)
+	f0();
+    }
+  return fail_count != 0 ? 1 : 0;
+}
+
+int
+main ()
+{
+  io io0;
+  f1 (&io0);
+  abort ();
+}
+
+void
+test (void)
+{
+  io *iop = i;
+  if (iop->i12.e0 != 8 || iop->i16.e0 != 8)
+    abort ();
+  exit (0);
+}
+
+void
+f0 (void)
+{
+  static int washere = 0;
+  io *iop = i;
+  if (washere++ || iop->i12.e0 != 32 || iop->i16.e0 != 32)
+    abort ();
+}

Index: resource.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/resource.c,v
retrieving revision 1.63
diff -p -c -r1.63 resource.c
*** resource.c	19 Jul 2003 14:47:13 -0000	1.63
--- resource.c	1 Dec 2003 04:53:48 -0000
*************** mark_set_resources (rtx x, struct resour
*** 800,808 ****
        return;
  
      case STRICT_LOW_PART:
!       if (! (mark_type == MARK_DEST && in_dest))
  	{
! 	  mark_set_resources (XEXP (x, 0), res, 0, MARK_SRC_DEST);
  	  return;
  	}
  
--- 800,809 ----
        return;
  
      case STRICT_LOW_PART:
!       /* Treat this similar to SUBREG.  */
!       if (in_dest)
  	{
! 	  mark_set_resources (XEXP (x, 0), res, in_dest, mark_type);
  	  return;
  	}
  



More information about the Gcc-patches mailing list