This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, MIPS] fix MIPS16 jump table overflow


On 08/21/2012 02:23 PM, Richard Sandiford wrote:

Would be nice to add a compile test for -mabi=64 just to make sure that Pmode == DImode works. A copy of an existing test like code-readable-1.c would be fine.

I'm having problems with this part -- it seems like every combination of options with -mabi=64 I've tried with code-readable-1.c complains about something-or-another being incompatible. The closest I've come is "-mabi=64 -march=mips64 -msoft-float", which is accepted by the mipsisa32r2-sde-elf build I'm using for testing, but when I add -O to that I get an unrelated ICE:


code-readable-1.c: In function 'bar':
code-readable-1.c:25:1: error: unrecognizable insn:
(insn 17 5 18 2 (set (reg/i:DI 2 $2)
(high:DI (const:DI (unspec:DI [
(symbol_ref:DI ("k") [flags 0x40] <var_decl 0xf72d14ac k>)
] 229)))) code-readable-1.c:25 -1
(nil))
code-readable-1.c:25:1: internal compiler error: in extract_insn, at recog.c:2125


And, even without -O, that combination of options gives a "sorry" on the mips-linux-gnu build I also have around.

Here's the revised patch that addresses the other problems you pointed out. Is this part OK, at least? It passes regression testing.

-Sandra


2012-08-22 Julian Brown <julian@codesourcery.com> Sandra Loosemore <sandra@codesourcery.com>

	gcc/
	* config/mips/mips.md
	(UNSPEC_CASESI_DISPATCH): New.
	(MIPS16_T_REGNUM): New constant.
	(tablejump): Don't use for MIPS16_SHORT_JUMP_TABLES.
	(casesi): New.
	(casesi_internal_mips16_<mode>): New.
	* config/mips/mips.c (mips16_split_long_branches): Adjust test
	to ignore casesi jump tables.
	* config/mips/mips.h (TARGET_MIPS16_SHORT_JUMP_TABLES): Update
	comment.
	(CASE_VECTOR_MODE): Use SImode unconditionally.
	(CASE_VECTOR_SHORTEN_MODE): Define.
	(ASM_OUTPUT_ADDR_DIFF_ELT): Output word-sized addr_diff_elts
	when necessary for MIPS16_SHORT_JUMP_TABLES.

	gcc/testsuite/
	* gcc.target/mips/code-readable-1.c: Add -O to options.

Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 190463)
+++ gcc/config/mips/mips.md	(working copy)
@@ -134,10 +134,14 @@
   ;; Used in a call expression in place of args_size.  It's present for PIC
   ;; indirect calls where it contains args_size and the function symbol.
   UNSPEC_CALL_ATTR
+
+  ;; MIPS16 casesi jump table dispatch.
+  UNSPEC_CASESI_DISPATCH
 ])
 
 (define_constants
   [(TLS_GET_TP_REGNUM		3)
+   (MIPS16_T_REGNUM		24)
    (PIC_FUNCTION_ADDR_REGNUM	25)
    (RETURN_ADDR_REGNUM		31)
    (CPRESTORE_SLOT_REGNUM	76)
@@ -5904,14 +5908,9 @@
   [(set (pc)
 	(match_operand 0 "register_operand"))
    (use (label_ref (match_operand 1 "")))]
-  ""
+  "!TARGET_MIPS16_SHORT_JUMP_TABLES"
 {
-  if (TARGET_MIPS16_SHORT_JUMP_TABLES)
-    operands[0] = expand_binop (Pmode, add_optab,
-				convert_to_mode (Pmode, operands[0], false),
-				gen_rtx_LABEL_REF (Pmode, operands[1]),
-				0, 0, OPTAB_WIDEN);
-  else if (TARGET_GPWORD)
+  if (TARGET_GPWORD)
     operands[0] = expand_binop (Pmode, add_optab, operands[0],
 				pic_offset_table_rtx, 0, 0, OPTAB_WIDEN);
   else if (TARGET_RTP_PIC)
@@ -5937,6 +5936,94 @@
   [(set_attr "type" "jump")
    (set_attr "mode" "none")])
 
+;; For MIPS16, we don't know whether a given jump table will use short or
+;; word-sized offsets until late in compilation, when we are able to determine
+;; the sizes of the insns which comprise the containing function.  This
+;; necessitates the use of the casesi rather than the tablejump pattern, since
+;; the latter tries to calculate the index of the offset to jump through early
+;; in compilation, i.e. at expand time, when nothing is known about the
+;; eventual function layout.
+
+(define_expand "casesi"
+  [(match_operand:SI 0 "register_operand" "")	; index to jump on
+   (match_operand:SI 1 "const_int_operand" "")	; lower bound
+   (match_operand:SI 2 "const_int_operand" "")	; total range
+   (match_operand 3 "" "")			; table label
+   (match_operand 4 "" "")]			; out of range label
+  "TARGET_MIPS16_SHORT_JUMP_TABLES"
+{
+  if (operands[1] != const0_rtx)
+    {
+      rtx reg = gen_reg_rtx (SImode);
+      rtx offset = gen_int_mode (-INTVAL (operands[1]), SImode);
+      
+      if (!arith_operand (offset, SImode))
+        offset = force_reg (SImode, offset);
+      
+      emit_insn (gen_addsi3 (reg, operands[0], offset));
+      operands[0] = reg;
+    }
+
+  if (!arith_operand (operands[0], SImode))
+    operands[0] = force_reg (SImode, operands[0]);
+
+  operands[2] = GEN_INT (INTVAL (operands[2]) + 1);
+
+  emit_jump_insn (PMODE_INSN (gen_casesi_internal_mips16,
+			      (operands[0], operands[2],
+			       operands[3], operands[4])));
+
+  DONE;
+})
+
+(define_insn "casesi_internal_mips16_<mode>"
+  [(set (pc)
+     (if_then_else
+       (leu (match_operand:SI 0 "register_operand" "d")
+	    (match_operand:SI 1 "arith_operand" "dI"))
+       (unspec:P
+        [(match_dup 0)
+	 (label_ref (match_operand 2 "" ""))]
+	UNSPEC_CASESI_DISPATCH)
+       (label_ref (match_operand 3 "" ""))))
+   (clobber (match_scratch:P 4 "=d"))
+   (clobber (match_scratch:P 5 "=d"))
+   (clobber (reg:SI MIPS16_T_REGNUM))]
+  "TARGET_MIPS16_SHORT_JUMP_TABLES"
+{
+  rtx diff_vec = PATTERN (next_real_insn (operands[2]));
+
+  gcc_assert (GET_CODE (diff_vec) == ADDR_DIFF_VEC);
+  
+  output_asm_insn ("sltu\t%0, %1", operands);
+  output_asm_insn ("bteqz\t%3", operands);
+  
+  switch (GET_MODE (diff_vec))
+    {
+    case HImode:
+      output_asm_insn ("sll\t%5, %0, 1", operands);
+      output_asm_insn ("la\t%4, %2", operands);
+      output_asm_insn ("<d>addu\t%5, %4, %5", operands);
+      output_asm_insn ("lh\t%5, 0(%5)", operands);
+      break;
+    
+    case SImode:
+      output_asm_insn ("sll\t%5, %0, 2", operands);
+      output_asm_insn ("la\t%4, %2", operands);
+      output_asm_insn ("<d>addu\t%5, %4, %5", operands);
+      output_asm_insn ("lw\t%5, 0(%5)", operands);
+      break;
+
+    default:
+      gcc_unreachable ();
+    }
+  
+  output_asm_insn ("addu\t%4, %4, %5", operands);
+  
+  return "j\t%4";
+}
+  [(set_attr "length" "32")])
+
 ;; For TARGET_USE_GOT, we save the gp in the jmp_buf as well.
 ;; While it is possible to either pull it off the stack (in the
 ;; o32 case) or recalculate it given t9 and our target label,
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	(revision 190463)
+++ gcc/config/mips/mips.c	(working copy)
@@ -15575,7 +15575,8 @@ mips16_split_long_branches (void)
       for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
 	if (JUMP_P (insn)
 	    && USEFUL_INSN_P (insn)
-	    && get_attr_length (insn) > 8)
+	    && get_attr_length (insn) > 8
+	    && (any_condjump_p (insn) || any_uncondjump_p (insn)))
 	  {
 	    rtx old_label, new_label, temp, saved_temp;
 	    rtx target, jump, jump_sequence;
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	(revision 190463)
+++ gcc/config/mips/mips.h	(working copy)
@@ -2330,13 +2330,18 @@ typedef struct mips_args {
 /* True if we're generating a form of MIPS16 code in which jump tables
    are stored in the text section and encoded as 16-bit PC-relative
    offsets.  This is only possible when general text loads are allowed,
-   since the table access itself will be an "lh" instruction.  */
-/* ??? 16-bit offsets can overflow in large functions.  */
+   since the table access itself will be an "lh" instruction.  If the
+   PC-relative offsets grow too large, 32-bit offsets are used instead.  */
 #define TARGET_MIPS16_SHORT_JUMP_TABLES TARGET_MIPS16_TEXT_LOADS
 
 #define JUMP_TABLES_IN_TEXT_SECTION TARGET_MIPS16_SHORT_JUMP_TABLES
 
-#define CASE_VECTOR_MODE (TARGET_MIPS16_SHORT_JUMP_TABLES ? HImode : ptr_mode)
+#define CASE_VECTOR_MODE SImode
+
+/* Only use short offsets if their range will not overflow.  */
+#define CASE_VECTOR_SHORTEN_MODE(MIN, MAX, BODY) \
+  (TARGET_MIPS16_SHORT_JUMP_TABLES && ((MIN) >= -32768 && (MAX) < 32768) \
+   ? HImode : SImode)
 
 #define CASE_VECTOR_PC_RELATIVE TARGET_MIPS16_SHORT_JUMP_TABLES
 
@@ -2636,8 +2641,14 @@ while (0)
 #define ASM_OUTPUT_ADDR_DIFF_ELT(STREAM, BODY, VALUE, REL)		\
 do {									\
   if (TARGET_MIPS16_SHORT_JUMP_TABLES)					\
-    fprintf (STREAM, "\t.half\t%sL%d-%sL%d\n",				\
-	     LOCAL_LABEL_PREFIX, VALUE, LOCAL_LABEL_PREFIX, REL);	\
+    {									\
+      if (GET_MODE (BODY) == HImode)					\
+	fprintf (STREAM, "\t.half\t%sL%d-%sL%d\n",			\
+		 LOCAL_LABEL_PREFIX, VALUE, LOCAL_LABEL_PREFIX, REL);	\
+      else								\
+	fprintf (STREAM, "\t.word\t%sL%d-%sL%d\n",			\
+		 LOCAL_LABEL_PREFIX, VALUE, LOCAL_LABEL_PREFIX, REL);	\
+    }									\
   else if (TARGET_GPWORD)						\
     fprintf (STREAM, "\t%s\t%sL%d\n",					\
 	     ptr_mode == DImode ? ".gpdword" : ".gpword",		\
Index: gcc/testsuite/gcc.target/mips/code-readable-1.c
===================================================================
--- gcc/testsuite/gcc.target/mips/code-readable-1.c	(revision 190463)
+++ gcc/testsuite/gcc.target/mips/code-readable-1.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options "(-mips16) -mcode-readable=yes -mgp32 addressing=absolute" } */
+/* { dg-options "(-mips16) -mcode-readable=yes -mgp32 addressing=absolute -O" } */
 
 MIPS16 int
 foo (int i)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]