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]

[PATCH, MIPS] fix MIPS16 jump table overflow


In config/mips/mips.h, there is presently this comment:

/* ??? 16-bit offsets can overflow in large functions.  */
#define TARGET_MIPS16_SHORT_JUMP_TABLES TARGET_MIPS16_TEXT_LOADS

A while ago we had a bug report where a big switch statement did, in fact, overflow the range of 16-bit offsets, causing a runtime error.

GCC already has generic machinery to shorten offset tables for switch statements that does the necessary range checking, but it only works with "casesi", not the lower-level "tablejump" expansion. So, this patch provides a "casesi" expander to handle this situation.

This patch has been in use on our local 4.5 and 4.6 branches for about a year now. When testing it on mainline, I found it tripped over the recent change to add MIPS16 branch overflow checking in other situations, causing it to get into an infinite loop. I think telling it to ignore these new jump insns it doesn't know how to process is the right thing to do, but I'm not sure if there's a better way to restrict the condition or make mips16_split_long_branches more robust. Richard,
since that's your code I assume you'll suggest an alternative if this doesn't meet with your approval. Is the rest of the patch OK to check in?


-Sandra


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

	gcc/
	* config/mips/mips.md (MIPS16_T_REGNUM): New constant.
	(tablejump): Don't use for MIPS16_SHORT_JUMP_TABLES.
	(casesi): New.
	(casesi_internal_mips16): New.
	* config/mips/mips.c (mips16_split_long_branches): Ignore
	casesi_internal_mips16 insns.
	* config/mips/mips.h (TARGET_MIPS16_SHORT_JUMP_TABLES): Update
	comment.
	(CASE_VECTOR_MODE): Use ptr_mode 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: Generalize assembler
	pattern	for jump table.
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 190463)
+++ gcc/config/mips/mips.md	(working copy)
@@ -138,6 +138,7 @@
 
 (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 +5905,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 +5933,91 @@
   [(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:SI 3 "" "")			; table label
+   (match_operand:SI 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 (gen_casesi_internal_mips16 (operands[0], operands[2],
+					      operands[3], operands[4]));
+
+  DONE;
+})
+
+(define_insn "casesi_internal_mips16"
+  [(set (pc)
+     (if_then_else
+       (leu (match_operand:SI 0 "register_operand" "d")
+	    (match_operand:SI 1 "arith_operand" "dI"))
+       (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
+			(label_ref (match_operand 2 "" ""))))
+       (label_ref (match_operand 3 "" ""))))
+   (clobber (match_scratch:SI 4 "=&d"))
+   (clobber (match_scratch:SI 5 "=d"))
+   (clobber (reg:SI MIPS16_T_REGNUM))
+   (use (label_ref (match_dup 2)))]
+  "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);
+  output_asm_insn ("la\t%4, %2", operands);
+  
+  switch (GET_MODE (diff_vec))
+    {
+    case HImode:
+      output_asm_insn ("sll\t%5, %0, 1", operands);
+      output_asm_insn ("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 ("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
+	    && INSN_CODE (insn) != CODE_FOR_casesi_internal_mips16)
 	  {
 	    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 ptr_mode
+
+/* 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 : ptr_mode)
 
 #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)
@@ -25,7 +25,7 @@ bar (void)
 }
 
 /* { dg-final { scan-assembler "\tla\t" } } */
-/* { dg-final { scan-assembler "\t\\.half\t" } } */
+/* { dg-final { scan-assembler "\t\\.\(half\|word\)\t\\.L\[0-9\]*-\\.L\[0-9\]\*" } } */
 /* { dg-final { scan-assembler-not "%hi\\(\[^)\]*L" } } */
 /* { dg-final { scan-assembler-not "%lo\\(\[^)\]*L" } } */
 

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