RFA: PR 34456: Use regs_invalidated_by_call in mark_set_resources

RFA: PR 34456: Use regs_invalidated_by_call in mark_set_resources

[Richard Sandiford]

This patch from Kyl Kylheku fixes PR 34456.  The GP register is
call-saved for MIPS n32 and n64, but because it is fixed, it is
also in call_used_regs.  resources.c would then think that the
register is dead after a call, allowing the GP restoration to
be put in a delay slot.

Tested on mips64-linux-gnu.  OK to install?

Richard

PS. I don't know if Kaz has a copyright assignment on file,
    but this appears to be his first patch, so it should
    be well within the limit.  (The testcase is mine.)


gcc/
200x-xx-xx  Kaz Kylheku  <kaz@zeugmasystems.com>

	PR rtl-optimization/34456
	* resources.c (mark_set_resources): Use regs_invalidated_by_call
	rather than call_used_regs and global_regs.

gcc/testsuite/
	PR rtl-optimization/34456
	* gcc.c-torture/execute/pr34456.c: New test.

Index: gcc/resource.c
===================================================================
--- gcc/resource.c	2007-12-16 09:35:56.000000000 +0000
+++ gcc/resource.c	2007-12-16 09:36:56.000000000 +0000
@@ -663,9 +663,8 @@ mark_set_resources (rtx x, struct resour
 	  rtx link;
 
 	  res->cc = res->memory = 1;
-	  for (r = 0; r < FIRST_PSEUDO_REGISTER; r++)
-	    if (call_used_regs[r] || global_regs[r])
-	      SET_HARD_REG_BIT (res->regs, r);
+
+	  IOR_HARD_REG_SET (res->regs, regs_invalidated_by_call);
 
 	  for (link = CALL_INSN_FUNCTION_USAGE (x);
 	       link; link = XEXP (link, 1))
Index: gcc/testsuite/gcc.c-torture/execute/pr34456.c
===================================================================
--- /dev/null	2007-12-15 09:13:22.548096750 +0000
+++ gcc/testsuite/gcc.c-torture/execute/pr34456.c	2007-12-16 09:35:50.000000000 +0000
@@ -0,0 +1,30 @@
+#include <stdlib.h>
+
+int __attribute__ ((noinline)) debug (void) { return 1; }
+int errors;
+
+struct s { int elt; int (*compare) (int); };
+
+static int
+compare (const void *x, const void *y)
+{
+  const struct s *s1 = x, *s2 = y;
+  int (*compare1) (int);
+  int elt2;
+
+  compare1 = s1->compare;
+  elt2 = s2->elt;
+  if (elt2 != 0 && debug () && compare1 (s1->elt) != 0)
+    errors++;
+  return compare1 (elt2);
+}
+
+int bad_compare (int x) { return -x; }
+struct s array[2] = { { 1, bad_compare }, { -1, bad_compare } };
+
+int
+main (void)
+{
+  qsort (array, 2, sizeof (struct s), compare);
+  return errors == 0;
+}

Re: RFA: PR 34456: Use regs_invalidated_by_call in mark_set_resources

[Paolo Bonzini]

> PS. I don't know if Kaz has a copyright assignment on file,
>     but this appears to be his first patch, so it should
>     be well within the limit.  (The testcase is mine.)

FYI the answers are no, and yes. :-)

Actually I don't think we include testcases in the size of "tiny 
changes" usually.

Paolo

Re: RFA: PR 34456: Use regs_invalidated_by_call in mark_set_resources

[Eric Botcazou]

> 200x-xx-xx  Kaz Kylheku  <kaz@zeugmasystems.com>
>
> 	PR rtl-optimization/34456
> 	* resources.c (mark_set_resources): Use regs_invalidated_by_call
> 	rather than call_used_regs and global_regs.
>
> gcc/testsuite/
> 	PR rtl-optimization/34456
> 	* gcc.c-torture/execute/pr34456.c: New test.

OK, thanks.

-- 
Eric Botcazou

Re: RFA: PR 34456: Use regs_invalidated_by_call in mark_set_resources

[Richard Sandiford]

Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>> 200x-xx-xx  Kaz Kylheku  <kaz@zeugmasystems.com>
>>
>> 	PR rtl-optimization/34456
>> 	* resources.c (mark_set_resources): Use regs_invalidated_by_call
>> 	rather than call_used_regs and global_regs.
>>
>> gcc/testsuite/
>> 	PR rtl-optimization/34456
>> 	* gcc.c-torture/execute/pr34456.c: New test.
>
> OK, thanks.

Thanks, now applied to trunk.  I forgot to say: this is a regression
from before MIPS GCC had explicit reloc support (3.3 and earlier),
so is it OK for the branches too, if testing goes OK?  (The same
test does fail for 4.2.  I haven't tried 4.1 yet, but that was
the version Kaz was using, so the bug definitely shows up for
4.1 somewhere.)

Richard

Re: RFA: PR 34456: Use regs_invalidated_by_call in mark_set_resources

[Eric Botcazou]

> Thanks, now applied to trunk.  I forgot to say: this is a regression
> from before MIPS GCC had explicit reloc support (3.3 and earlier),
> so is it OK for the branches too, if testing goes OK?

Yes, it looks safe enough.

-- 
Eric Botcazou