Patchwork [MIPS,committed] Give MIPS16 instruction lengths directly

login
register
mail settings
Submitter Richard Sandiford
Date May 19, 2013, 10:15 a.m.
Message ID <87txlzp960.fsf@talisman.default>
Download mbox | patch
Permalink /patch/244798/
State New
Headers show

Comments

Richard Sandiford - May 19, 2013, 10:15 a.m.
The .md "length" attributes for MIPS16 instructions have traditionally
been double their real values, with a compensating:

  if (TARGET_MIPS16)
    length /= 2;

in ADJUST_INSN_LENGTH.  This was probably the most expedient way of
doing things way back when, because many patterns specified the
length directly.  However, these days the length is often inferred
from other attributes like extended_mips16, and this doubling trick
probably hinders more than it helps.  E.g. I notice that we don't
double the size of the constant table patterns, so I think they ended
up being too small.  Doubling also made the branch calculations more
confusing, since the ranges had to be specified in real units,
but the resulting branch lengths had to be specified in doubled units.

This patch instead specifies the real lengths directly.  This involves
adding a macro to give the length of a nop insn, and since microMIPS
also has 16-bit NOPs, I used TARGET_COMPRESSION rather than TARGET_MIPS16.
Also, the JR in a microMIPS long branch sequence is 2 bytes rather than 4.

Tested on mipsisa32-sde-elf and applied.

Richard


gcc/
	* config/mips/mips.h (BASE_INSN_LENGTH, NOP_INSN_LENGTH): New macros.
	* config/mips/mips.c (mips_symbol_insns, mips_address_insns)
	(mips_const_insns, mips_split_const_insns, mips_load_store_insns)
	(mips_idiv_insns): Update the comments to say that the returned
	instruction counts are in units of BASE_INSN_LENGTH.
	(mips_adjust_insn_length): Multiply the mips_load_label_num_insns
	by BASE_INSN_LENGTH rather than 4.  Add the jump separately,
	using 2 rather than 4 as the length of indirect MIPS16 and
	microMIPS jumps.  Use NOP_INSN_LENGTH rather than 4 as the
	length of a NOP.  Don't divide MIPS16 lengths by 2.
	(mips16_split_long_branches): Assume a branch is long if the
	length is greater than 4 rather than 8.
	* config/mips/mips.md (length): Give MIPS16 lengths directly,
	rather than multiplying them by 2.  Multiply instruction counts
	by BASE_INSN_LENGTH rather than 4.
	(*jump_mips16, tls_get_tp_mips16_<mode>)
	(*tls_get_tp_mips16_call_<mode>): Divide lengths by 2.

Patch

Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	2013-05-19 11:05:09.987763980 +0100
+++ gcc/config/mips/mips.h	2013-05-19 11:10:32.138190186 +0100
@@ -2439,6 +2439,14 @@  #define AVOID_CCMODE_COPIES
 #define BRANCH_COST(speed_p, predictable_p) mips_branch_cost
 #define LOGICAL_OP_NON_SHORT_CIRCUIT 0
 
+/* The MIPS port has several functions that return an instruction count.
+   Multiplying the count by this value gives the number of bytes that
+   the instructions occupy.  */
+#define BASE_INSN_LENGTH (TARGET_MIPS16 ? 2 : 4)
+
+/* The length of a NOP in bytes.  */
+#define NOP_INSN_LENGTH (TARGET_COMPRESSION ? 2 : 4)
+
 /* If defined, modifies the length assigned to instruction INSN as a
    function of the context in which it is used.  LENGTH is an lvalue
    that contains the initially computed length of the insn and should
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2013-05-19 11:05:09.987763980 +0100
+++ gcc/config/mips/mips.c	2013-05-19 11:10:32.137190179 +0100
@@ -2007,7 +2007,7 @@  mips_symbol_insns_1 (enum mips_symbol_ty
    values of mode MODE to or from addresses of type TYPE.  Return 0 if
    the given type of symbol is not valid in addresses.
 
-   In both cases, treat extended MIPS16 instructions as two instructions.  */
+   In both cases, instruction counts are based off BASE_INSN_LENGTH.  */
 
 static int
 mips_symbol_insns (enum mips_symbol_type type, enum machine_mode mode)
@@ -2334,12 +2334,11 @@  mips16_unextended_reference_p (enum mach
 }
 
 /* Return the number of instructions needed to load or store a value
-   of mode MODE at address X.  Return 0 if X isn't valid for MODE.
+   of mode MODE at address X, assuming that BASE_INSN_LENGTH is the
+   length of one instruction.  Return 0 if X isn't valid for MODE.
    Assume that multiword moves may need to be split into word moves
    if MIGHT_SPLIT_P, otherwise assume that a single load or store is
-   enough.
-
-   For MIPS16 code, count extended instructions as two instructions.  */
+   enough.  */
 
 int
 mips_address_insns (rtx x, enum machine_mode mode, bool might_split_p)
@@ -2441,7 +2440,8 @@  umips_12bit_offset_address_p (rtx x, enu
 	  && UMIPS_12BIT_OFFSET_P (INTVAL (addr.offset)));
 }
 
-/* Return the number of instructions needed to load constant X.
+/* Return the number of instructions needed to load constant X,
+   assuming that BASE_INSN_LENGTH is the length of one instruction.
    Return 0 if X isn't a valid constant.  */
 
 int
@@ -2524,7 +2524,8 @@  mips_const_insns (rtx x)
 
 /* X is a doubleword constant that can be handled by splitting it into
    two words and loading each word separately.  Return the number of
-   instructions required to do this.  */
+   instructions required to do this, assuming that BASE_INSN_LENGTH
+   is the length of one instruction.  */
 
 int
 mips_split_const_insns (rtx x)
@@ -2538,8 +2539,8 @@  mips_split_const_insns (rtx x)
 }
 
 /* Return the number of instructions needed to implement INSN,
-   given that it loads from or stores to MEM.  Count extended
-   MIPS16 instructions as two instructions.  */
+   given that it loads from or stores to MEM.  Assume that
+   BASE_INSN_LENGTH is the length of one instruction.  */
 
 int
 mips_load_store_insns (rtx mem, rtx insn)
@@ -2563,7 +2564,8 @@  mips_load_store_insns (rtx mem, rtx insn
   return mips_address_insns (XEXP (mem, 0), mode, might_split_p);
 }
 
-/* Return the number of instructions needed for an integer division.  */
+/* Return the number of instructions needed for an integer division,
+   assuming that BASE_INSN_LENGTH is the length of one instruction.  */
 
 int
 mips_idiv_insns (void)
@@ -12273,16 +12275,18 @@  mips_adjust_insn_length (rtx insn, int l
 	 is a conditional branch.  */
       length = simplejump_p (insn) ? 0 : 8;
 
-      /* Load the label into $AT and jump to it.  Ignore the delay
-	 slot of the jump.  */
-      length += 4 * mips_load_label_num_insns() + 4;
+      /* Add the size of a load into $AT.  */
+      length += BASE_INSN_LENGTH * mips_load_label_num_insns ();
+
+      /* Add the length of an indirect jump, ignoring the delay slot.  */
+      length += TARGET_COMPRESSION ? 2 : 4;
     }
 
   /* A unconditional jump has an unfilled delay slot if it is not part
      of a sequence.  A conditional jump normally has a delay slot, but
      does not on MIPS16.  */
   if (CALL_P (insn) || (TARGET_MIPS16 ? simplejump_p (insn) : JUMP_P (insn)))
-    length += 4;
+    length += TARGET_MIPS16 ? 2 : 4;
 
   /* See how many nops might be needed to avoid hardware hazards.  */
   if (!cfun->machine->ignore_hazard_length_p && INSN_CODE (insn) >= 0)
@@ -12292,20 +12296,14 @@  mips_adjust_insn_length (rtx insn, int l
 	break;
 
       case HAZARD_DELAY:
-	length += 4;
+	length += NOP_INSN_LENGTH;
 	break;
 
       case HAZARD_HILO:
-	length += 8;
+	length += NOP_INSN_LENGTH * 2;
 	break;
       }
 
-  /* In order to make it easier to share MIPS16 and non-MIPS16 patterns,
-     the .md file length attributes are 4-based for both modes.
-     Adjust the MIPS16 ones here.  */
-  if (TARGET_MIPS16)
-    length /= 2;
-
   return length;
 }
 
@@ -16201,7 +16199,7 @@  mips16_split_long_branches (void)
       something_changed = false;
       for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
 	if (JUMP_P (insn)
-	    && get_attr_length (insn) > 8
+	    && get_attr_length (insn) > 4
 	    && (any_condjump_p (insn) || any_uncondjump_p (insn)))
 	  {
 	    rtx old_label, new_label, temp, saved_temp;
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2013-05-19 11:05:09.987763980 +0100
+++ gcc/config/mips/mips.md	2013-05-19 11:11:29.059618158 +0100
@@ -425,7 +425,7 @@  (define_attr "enabled" "no,yes"
 (define_attr "length" ""
    (cond [(and (eq_attr "extended_mips16" "yes")
 	       (match_test "TARGET_MIPS16"))
-	  (const_int 8)
+	  (const_int 4)
 
 	  (and (eq_attr "compression" "micromips,all")
 	       (eq_attr "dword_mode" "no")
@@ -567,68 +567,69 @@  (define_attr "length" ""
 	  ;;	 move $2,$1		2 bytes
 	  ;; foo:
 	  ;;				(20 bytes in the worst case)
-	  ;;
-	  ;; Note that the conditions test adjusted lengths, whereas the
-	  ;; result is an unadjusted length, and is thus twice the true value.
 	  (and (eq_attr "type" "branch")
 	       (match_test "TARGET_MIPS16"))
 	  (cond [(and (le (minus (match_dup 0) (pc)) (const_int 254))
 		      (le (minus (pc) (match_dup 0)) (const_int 254)))
-		 (const_int 4)
+		 (const_int 2)
 		 (and (le (minus (match_dup 0) (pc)) (const_int 65534))
 		      (le (minus (pc) (match_dup 0)) (const_int 65532)))
-		 (const_int 8)
+		 (const_int 4)
 		 (and (match_test "TARGET_ABICALLS")
 		      (not (match_test "TARGET_ABSOLUTE_ABICALLS")))
-		 (const_int 40)
+		 (const_int 20)
 		 (match_test "Pmode == SImode")
-		 (const_int 32)
-		 ] (const_int 48))
+		 (const_int 16)
+		 ] (const_int 24))
 
 	  ;; "Ghost" instructions occupy no space.
 	  (eq_attr "type" "ghost")
 	  (const_int 0)
 
+	  ;; GOT loads are extended MIPS16 instructions and 4-byte
+	  ;; microMIPS instructions.
 	  (eq_attr "got" "load")
-	  (if_then_else (match_test "TARGET_MIPS16")
-			(const_int 8)
-			(const_int 4))
+	  (const_int 4)
+
+	  ;; A GOT load followed by an add of $gp.
 	  (eq_attr "got" "xgot_high")
 	  (const_int 8)
 
 	  ;; In general, constant-pool loads are extended instructions.
 	  (eq_attr "move_type" "loadpool")
-	  (const_int 8)
+	  (const_int 4)
 
 	  ;; SHIFT_SHIFTs are decomposed into two separate instructions.
 	  ;; They are extended instructions on MIPS16 targets.
 	  (eq_attr "move_type" "shift_shift")
-	  (if_then_else (match_test "TARGET_MIPS16")
-			(const_int 16)
-			(const_int 8))
+	  (const_int 8)
 
 	  ;; Check for doubleword moves that are decomposed into two
-	  ;; instructions.
+	  ;; instructions.  The individual instructions are unextended
+	  ;; MIPS16 ones or 2-byte microMIPS ones.
 	  (and (eq_attr "move_type" "mtc,mfc,mtlo,mflo,move")
 	       (eq_attr "dword_mode" "yes"))
-	  (const_int 8)
+	  (if_then_else (match_test "TARGET_COMPRESSION")
+	  		(const_int 4)
+	  		(const_int 8))
 
 	  ;; Doubleword CONST{,N} moves are split into two word
 	  ;; CONST{,N} moves.
 	  (and (eq_attr "move_type" "const,constN")
 	       (eq_attr "dword_mode" "yes"))
-	  (symbol_ref "mips_split_const_insns (operands[1]) * 4")
+	  (symbol_ref "mips_split_const_insns (operands[1]) * BASE_INSN_LENGTH")
 
 	  ;; Otherwise, constants, loads and stores are handled by external
 	  ;; routines.
 	  (eq_attr "move_type" "const,constN")
-	  (symbol_ref "mips_const_insns (operands[1]) * 4")
+	  (symbol_ref "mips_const_insns (operands[1]) * BASE_INSN_LENGTH")
 	  (eq_attr "move_type" "load,fpload")
-	  (symbol_ref "mips_load_store_insns (operands[1], insn) * 4")
+	  (symbol_ref "mips_load_store_insns (operands[1], insn)
+	  	       * BASE_INSN_LENGTH")
 	  (eq_attr "move_type" "store,fpstore")
-	  (cond [(not (match_test "TARGET_FIX_24K"))
-	         (symbol_ref "mips_load_store_insns (operands[0], insn) * 4")]
-	         (symbol_ref "mips_load_store_insns (operands[0], insn) * 4 + 4"))
+	  (symbol_ref "mips_load_store_insns (operands[0], insn)
+		       * BASE_INSN_LENGTH
+		       + (TARGET_FIX_24K ? NOP_INSN_LENGTH : 0)")
 
 	  ;; In the worst case, a call macro will take 8 instructions:
 	  ;;
@@ -659,10 +660,14 @@  (define_attr "length" ""
 	  (const_int 8)
 
 	  (eq_attr "type" "idiv,idiv3")
-	  (symbol_ref "mips_idiv_insns () * 4")
+	  (symbol_ref "mips_idiv_insns () * BASE_INSN_LENGTH")
 
 	  (not (eq_attr "sync_mem" "none"))
-	  (symbol_ref "mips_sync_loop_insns (insn, operands) * 4")
+	  (symbol_ref "mips_sync_loop_insns (insn, operands)
+	  	       * BASE_INSN_LENGTH")
+
+	  (match_test "TARGET_MIPS16")
+	  (const_int 2)
 	  ] (const_int 4)))
 
 ;; Attribute describing the processor.
@@ -5373,7 +5378,7 @@  (define_insn_and_split ""
   ""
   [(set_attr "type"	"load")
    (set_attr "mode"	"SI")
-   (set_attr "length"	"16")])
+   (set_attr "length"	"8")])
 
 (define_insn "rotr<mode>3"
   [(set (match_operand:GPR 0 "register_operand" "=d")
@@ -5845,16 +5850,16 @@  (define_insn "*jump_mips16"
 	;; is one instruction shorter than for conditional branches.
 	(cond [(and (le (minus (match_dup 0) (pc)) (const_int 2046))
 		    (le (minus (pc) (match_dup 0)) (const_int 2046)))
-	       (const_int 4)
+	       (const_int 2)
 	       (and (le (minus (match_dup 0) (pc)) (const_int 65534))
 		    (le (minus (pc) (match_dup 0)) (const_int 65532)))
-	       (const_int 8)
+	       (const_int 4)
 	       (and (match_test "TARGET_ABICALLS")
 		    (not (match_test "TARGET_ABSOLUTE_ABICALLS")))
-	       (const_int 36)
+	       (const_int 18)
 	       (match_test "Pmode == SImode")
-	       (const_int 28)
-	       ] (const_int 44)))])
+	       (const_int 14)
+	       ] (const_int 22)))])
 
 (define_expand "indirect_jump"
   [(set (pc) (match_operand 0 "register_operand"))]
@@ -6947,7 +6952,7 @@  (define_insn_and_split "tls_get_tp_mips1
    (set (match_dup 0) (reg:P TLS_GET_TP_REGNUM))]
   ""
   [(set_attr "type" "multi")
-   (set_attr "length" "16")
+   (set_attr "length" "8")
    (set_attr "mode" "<MODE>")])
 
 (define_insn "*tls_get_tp_mips16_call_<mode>"
@@ -6959,7 +6964,7 @@  (define_insn "*tls_get_tp_mips16_call_<m
   "HAVE_AS_TLS && TARGET_MIPS16"
   { return MIPS_CALL ("jal", operands, 0, -1); }
   [(set_attr "type" "call")
-   (set_attr "length" "12")
+   (set_attr "length" "6")
    (set_attr "mode" "<MODE>")])
 
 ;; Named pattern for expanding thread pointer reference.