[MIPS][LS2][4/5] Scheduling and tuning

[Richard Sandiford]

Maxim Kuvyrkov <maxim@codesourcery.com> writes:
> Maxim Kuvyrkov wrote:
>> Richard Sandiford wrote:
>
> ...
>
>>> Anyway, this patch looks good, thanks.  A neat use of CPU querying ;)
>
> I believe this patch exposes a latent bug in MIPS backend.
>
> Though the bug is visible only when scheduling for Loongson, my 
> investigation suggests that it can occur on other MIPS architectures too.
>
> Here is the error:
>
> <snip>
> ./cc1 -quiet -march=loongson2e -O2 -o testcase.s testcase.c
> testcase.c: In function '_nl_normalize_codeset':
> testcase.c:211: error: unable to find a register to spill in class 'V1_REG'
> testcase.c:211: error: this is the insn:
> (insn 51 45 47 3 testcase.c:150 (set (reg:SI 11 $11 [279])
>          (unspec:SI [
>                  (const_int 0 [0x0])
>              ] 30)) 570 {tls_get_tp_si} (expr_list:REG_EQUIV (unspec:SI [
>                  (const_int 0 [0x0])
>              ] 30)
>          (nil)))
> testcase.c:211: internal compiler error: in spill_failure, at reload1.c:1999
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> </snip>

FWIW, this is PR 35802.  'Fraid I still haven't had chance to look at it,
what with IRA, recog-related stuff and reviews.

> The error is triggered by two consecutive tls_get_tp_si instructions. 
> These instructions use "v" constraint, which specifies V1_REG, aka $3.
>
> If no scheduling is performed, then the two tls_get_tp_si instructions 
> are separated by one instruction; when the scheduler is on, they end up 
> together and reload crashes.
>
> In the case when two get_tp instructions are separated by a third 
> instruction, reload manages to remove the second get_tp instruction 
> (instructions are the same, so only one of them is needed) and 
> everything works out well.  If the two instructions are side-by-side 
> reload can't find a spill register for the second get_tp and crashes.
>
> My speculation is that mov<mode>_internal should be taught how to move 
> "v" into "d".  RichardS should know for sure.

It already knows how to do that.  "v" is a subclass of "d", so it's
just a normal GPR<->GPR move.

The problem is that "v" represents a single register: $3.  We initially
generate these instructions with $3 as the result, but the insn predicates
allow it to be replaced by any register_operand later.  Thus we can end
up with a pseudo register that is:

  (a) the output operand of a tls_get_tp_<mode> insn, and must thus
      be reloaded into $3;
  (b) live at the same time as an explicit $3.

We have two choices:

  - always use pseudo registers, rather than introducing uses of $3
    from the outset

  - force the destination of tls_get_tp_<mode> to be $3 only.

The second is probably the most conservative approach, since explicit
uses of $3 can occur through normal calls.  The patch below does this.

I admit I haven't verified any of this yet, so sorry if I'm off-ball.
But does the patch fix things?

Richard


gcc/
	* config/mips/predicates.md (v1_operand): New predicate.
	* config/mips/mips.md (tls_get_tp_<mode>): Use it instead of
	register_operand.

Index: gcc/config/mips/predicates.md
===================================================================
--- gcc/config/mips/predicates.md	2008-06-12 18:54:01.000000000 +0100
+++ gcc/config/mips/predicates.md	2008-06-12 19:03:27.000000000 +0100
@@ -76,6 +76,10 @@ (define_predicate "const_0_or_1_operand"
        (ior (match_test "op == CONST0_RTX (GET_MODE (op))")
 	    (match_test "op == CONST1_RTX (GET_MODE (op))"))))
 
+(define_predicate "v1_operand"
+  (and (match_code "reg")
+       (match_test "REGNO (op) == GP_RETURN + 1")))
+
 (define_predicate "d_operand"
   (and (match_code "reg")
        (match_test "TARGET_MIPS16
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2008-06-12 18:53:41.000000000 +0100
+++ gcc/config/mips/mips.md	2008-06-12 18:53:49.000000000 +0100
@@ -6410,7 +6410,7 @@ (define_insn "*mips16e_save_restore"
 ; accept it.
 
 (define_insn "tls_get_tp_<mode>"
-  [(set (match_operand:P 0 "register_operand" "=v")
+  [(set (match_operand:P 0 "v1_operand" "=v")
 	(unspec:P [(const_int 0)]
 		  UNSPEC_TLS_GET_TP))]
   "HAVE_AS_TLS && !TARGET_MIPS16"