[tree->rtl] Fix execute/20041124-1.c for SH PIC

[tree->rtl] Fix execute/20041124-1.c for SH PIC

[Richard Sandiford]

This patch fixes an sh-elf ICE on gcc.c-torture/execute/20041124-1.c
for -O -fPIC.  The testcase is simple:

-------------------------------------------------------------------
struct s { _Complex unsigned short x; };
struct s gs = { 100 + 200i };
struct s __attribute__((noinline)) foo (void) { return gs; }

int main ()
{
  if (foo ().x != gs.x)
    abort ();
  exit (0);
}
-------------------------------------------------------------------

foo() returns in (reg:CHI r0).  Because r0 is a likely-spilled register,
we would normally try to copy it into a pseudo register immediately
after a call to foo().  Passes like combine would then check for such
a move and refuse to propagate r0 into later instructions, avoiding the
risk of a spill failure.

Unfortunately, CHI is special.  The "pseudo register" we create is
actually a CONCAT of two HImode pseudo registers, and the assigment
to the high part is an lshiftrt of r0 rather than a simple move.
This assignment therefore avoids the special checks done in combine,
and we try to combine the lshiftrt into later instructions.  In the test
case, we extend r0's lifetime across a GOT load, and the GOT load must
use r0 for the index.

The two obvious fixes seem to be: make combine smarter, or avoid using
a likely-spilled return register on the rhs of the CONCAT assignment.
The latter seemed better to me, as it should work with any similar
checks in passes besides combine.

It wasn't safe at one time to create a true CHI pseudo register, but
rth's emit_move_insn_1/emit_move_complex_parts improvements should have
fixed that.  The patch therefore copies a likely-spilled hard register
to a non-CONCAT register.

Bootstrapped & regression-tested on x86_64-linux-gnu.
Also regression-tested on i586-wrs-vxworks, sh-wrs-vxworks
and sh-elf (options {,-m2,-m3,-m4,-m4/-ml}).  OK to install?

Richard


gcc/
	* calls.c (avoid_likely_spilled_reg): New function.
	(expand_call): Use it.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 127322)
+++ gcc/calls.c	(working copy)
@@ -1856,6 +1856,31 @@ shift_return_value (enum machine_mode mo
   return true;
 }
 
+/* If X is a likely-spilled register value, copy it to a pseudo
+   register and return that register.  Return X otherwise.  */
+
+static rtx
+avoid_likely_spilled_reg (rtx x)
+{
+  rtx new;
+
+  if (REG_P (x)
+      && HARD_REGISTER_P (x)
+      && CLASS_LIKELY_SPILLED_P (REGNO_REG_CLASS (REGNO (x))))
+    {
+      /* Make sure that we generate a REG rather than a CONCAT.
+	 Moves into CONCATs can need nontrivial instructions,
+	 and the whole point of this function is to avoid
+	 using the hard register directly in such a situation.  */
+      generating_concat_p = 0;
+      new = gen_reg_rtx (GET_MODE (x));
+      generating_concat_p = 1;
+      emit_move_insn (new, x);
+      return new;
+    }
+  return x;
+}
+
 /* Generate all the code for a CALL_EXPR exp
    and return an rtx for its value.
    Store the value in TARGET (specified as an rtx) if convenient.
@@ -2953,11 +2978,7 @@ expand_call (tree exp, rtx target, int i
 
 	  /* We have to copy a return value in a CLASS_LIKELY_SPILLED hard
 	     reg to a plain register.  */
-	  if (REG_P (valreg)
-	      && HARD_REGISTER_P (valreg)
-	      && CLASS_LIKELY_SPILLED_P (REGNO_REG_CLASS (REGNO (valreg)))
-	      && !(REG_P (target) && !HARD_REGISTER_P (target)))
-	    valreg = copy_to_reg (valreg);
+	  valreg = avoid_likely_spilled_reg (valreg);
 
 	  /* If TARGET is a MEM in the argument area, and we have
 	     saved part of the argument area, then we can't store
@@ -3002,7 +3023,7 @@ expand_call (tree exp, rtx target, int i
 	  sibcall_failure = 1;
 	}
       else
-	target = copy_to_reg (valreg);
+	target = copy_to_reg (avoid_likely_spilled_reg (valreg));
 
       if (targetm.calls.promote_function_return(funtype))
 	{

Re: [tree->rtl] Fix execute/20041124-1.c for SH PIC

[Ian Lance Taylor]

Richard Sandiford <richard@codesourcery.com> writes:

> -	  if (REG_P (valreg)
> -	      && HARD_REGISTER_P (valreg)
> -	      && CLASS_LIKELY_SPILLED_P (REGNO_REG_CLASS (REGNO (valreg)))
> -	      && !(REG_P (target) && !HARD_REGISTER_P (target)))
> -	    valreg = copy_to_reg (valreg);
> +	  valreg = avoid_likely_spilled_reg (valreg);

You've dropped the test of whether TARGET is a hard register.  Why?
It's probably OK to drop that when you have a complex mode, but you've
dropped it in all cases.

Ian

Re: [tree->rtl] Fix execute/20041124-1.c for SH PIC

[Richard Sandiford]

Ian Lance Taylor <iant@google.com> writes:
> Richard Sandiford <richard@codesourcery.com> writes:
>> -	  if (REG_P (valreg)
>> -	      && HARD_REGISTER_P (valreg)
>> -	      && CLASS_LIKELY_SPILLED_P (REGNO_REG_CLASS (REGNO (valreg)))
>> -	      && !(REG_P (target) && !HARD_REGISTER_P (target)))
>> -	    valreg = copy_to_reg (valreg);
>> +	  valreg = avoid_likely_spilled_reg (valreg);
>
> You've dropped the test of whether TARGET is a hard register.  Why?
> It's probably OK to drop that when you have a complex mode, but you've
> dropped it in all cases.

Plain carelessness, sorry.  I meant to keep that check when I was
weighing up this approach vs the combine one, but forgot when
I wrote the patch.

Is the version below OK?  Re-bootstrapped & regression-tested on
x86_64-linux-gnu, and regression-tested on sh-elf.

Richard


gcc/
	* calls.c (avoid_likely_spilled_reg): New function.
	(expand_call): Use it.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 127336)
+++ gcc/calls.c	(working copy)
@@ -1856,6 +1856,31 @@ shift_return_value (enum machine_mode mo
   return true;
 }
 
+/* If X is a likely-spilled register value, copy it to a pseudo
+   register and return that register.  Return X otherwise.  */
+
+static rtx
+avoid_likely_spilled_reg (rtx x)
+{
+  rtx new;
+
+  if (REG_P (x)
+      && HARD_REGISTER_P (x)
+      && CLASS_LIKELY_SPILLED_P (REGNO_REG_CLASS (REGNO (x))))
+    {
+      /* Make sure that we generate a REG rather than a CONCAT.
+	 Moves into CONCATs can need nontrivial instructions,
+	 and the whole point of this function is to avoid
+	 using the hard register directly in such a situation.  */
+      generating_concat_p = 0;
+      new = gen_reg_rtx (GET_MODE (x));
+      generating_concat_p = 1;
+      emit_move_insn (new, x);
+      return new;
+    }
+  return x;
+}
+
 /* Generate all the code for a CALL_EXPR exp
    and return an rtx for its value.
    Store the value in TARGET (specified as an rtx) if convenient.
@@ -2953,11 +2978,8 @@ expand_call (tree exp, rtx target, int i
 
 	  /* We have to copy a return value in a CLASS_LIKELY_SPILLED hard
 	     reg to a plain register.  */
-	  if (REG_P (valreg)
-	      && HARD_REGISTER_P (valreg)
-	      && CLASS_LIKELY_SPILLED_P (REGNO_REG_CLASS (REGNO (valreg)))
-	      && !(REG_P (target) && !HARD_REGISTER_P (target)))
-	    valreg = copy_to_reg (valreg);
+	  if (!REG_P (target) || HARD_REGISTER_P (target))
+	    valreg = avoid_likely_spilled_reg (valreg);
 
 	  /* If TARGET is a MEM in the argument area, and we have
 	     saved part of the argument area, then we can't store
@@ -3002,7 +3024,7 @@ expand_call (tree exp, rtx target, int i
 	  sibcall_failure = 1;
 	}
       else
-	target = copy_to_reg (valreg);
+	target = copy_to_reg (avoid_likely_spilled_reg (valreg));
 
       if (targetm.calls.promote_function_return(funtype))
 	{

Re: [tree->rtl] Fix execute/20041124-1.c for SH PIC

[Ian Lance Taylor]

Richard Sandiford <richard@codesourcery.com> writes:

> Is the version below OK?  Re-bootstrapped & regression-tested on
> x86_64-linux-gnu, and regression-tested on sh-elf.
> 
> Richard
> 
> 
> gcc/
> 	* calls.c (avoid_likely_spilled_reg): New function.
> 	(expand_call): Use it.

This is OK.

Thanks.

Ian