diff mbox

[M68K] allow long offsets in jump tables (wrong-code PR target/57583)

Message ID 22639.49394.656974.300679@gargle.gargle.HOWL
State New
Headers show

Commit Message

Mikael Pettersson Jan. 6, 2017, 4:08 p.m. UTC 
This fixes / works-around the wrong-code PR57583 on M68K, caused by
overflowing the 16-bit jump table offsets the backend uses.

Ideally the backend should define CASE_VECTOR_SHORTEN_MODE, but that
AFAIK needs insn length attributes, which the backend only has for CF
but not for classic M68K.

Instead this patch adds an -mlong-jump-table-offsets option, and adjusts
the code for emitting and using jump table offsets to handle both short
and long offsets.  As long as the option is not selected, the backend
behaves exactly as before.

John Paul Adrian Glaubitz tested this patch by compiling "mednafen"
with it, which previously failed; I also tested earlier versions.

Is this Ok for trunk?

/Mikael


gcc/

2017-01-06  Mikael Pettersson  <mikpelinux@gmail.com>

	PR target/57583
	* config/m68k/m68k.opt (LONG_JUMP_TABLE_OFFSETS): New option.
	* config/m68k/linux.h (ASM_RETURN_CASE_JUMP): Handle
	TARGET_LONG_JUMP_TABLE_OFFSETS.
	* config/m68k/m68kelf.h (ASM_RETURN_CASE_JUMP): Likewise.
	* config/m68k/netbsd-elf.h (ASM_RETURN_CASE_JUMP): Likewise.
	* config/m68k/m68k.h (CASE_VECTOR_MODE): Likewise.
	(ASM_OUTPUT_ADDR_DIFF_ELF): Likewise.
	* config/m68k/m68k.md (tablejump expander): Likewise.
	(*tablejump_pcrel_hi): Renamed from unnamed insn, reject
	TARGET_LONG_JUMP_TABLE_OFFSETS.
	(*tablejump_pcrel_si): New insn, handle TARGET_LONG_JUMP_TABLE_OFFSETS.
	* doc/invoke.texi (M68K options): Add -mlong-jump-table-offsets.

Comments

Jeff Law Jan. 6, 2017, 5:22 p.m. UTC | #1 
On 01/06/2017 09:08 AM, Mikael Pettersson wrote:
> This fixes / works-around the wrong-code PR57583 on M68K, caused by
> overflowing the 16-bit jump table offsets the backend uses.
>
> Ideally the backend should define CASE_VECTOR_SHORTEN_MODE, but that
> AFAIK needs insn length attributes, which the backend only has for CF
> but not for classic M68K.
>
> Instead this patch adds an -mlong-jump-table-offsets option, and adjusts
> the code for emitting and using jump table offsets to handle both short
> and long offsets.  As long as the option is not selected, the backend
> behaves exactly as before.
>
> John Paul Adrian Glaubitz tested this patch by compiling "mednafen"
> with it, which previously failed; I also tested earlier versions.
>
> Is this Ok for trunk?
>
> /Mikael
>
>
> gcc/
>
> 2017-01-06  Mikael Pettersson  <mikpelinux@gmail.com>
>
> 	PR target/57583
> 	* config/m68k/m68k.opt (LONG_JUMP_TABLE_OFFSETS): New option.
> 	* config/m68k/linux.h (ASM_RETURN_CASE_JUMP): Handle
> 	TARGET_LONG_JUMP_TABLE_OFFSETS.
> 	* config/m68k/m68kelf.h (ASM_RETURN_CASE_JUMP): Likewise.
> 	* config/m68k/netbsd-elf.h (ASM_RETURN_CASE_JUMP): Likewise.
> 	* config/m68k/m68k.h (CASE_VECTOR_MODE): Likewise.
> 	(ASM_OUTPUT_ADDR_DIFF_ELF): Likewise.
> 	* config/m68k/m68k.md (tablejump expander): Likewise.
> 	(*tablejump_pcrel_hi): Renamed from unnamed insn, reject
> 	TARGET_LONG_JUMP_TABLE_OFFSETS.
> 	(*tablejump_pcrel_si): New insn, handle TARGET_LONG_JUMP_TABLE_OFFSETS.
> 	* doc/invoke.texi (M68K options): Add -mlong-jump-table-offsets.
Can't tablejump_pcrel_si be simplified?  Isn't the output pattern just:

#ifdef ASM_RETURN_CASE_JUMP
   ASM_RETURN_CASE_JUMP;
#else
   return MOTOROLA ? "jmp (2,pc,%0.l)" : "jmp pc@(2,%0:l)";
#endif

With that simplification I think this will be fine.

jeff
diff mbox

Patch

--- gcc-7-20170101/gcc/config/m68k/linux.h.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/config/m68k/linux.h	2017-01-06 14:36:37.366245875 +0100
@@ -98,9 +98,13 @@  along with GCC; see the file COPYING3.
       {							\
 	if (ADDRESS_REG_P (operands[0]))		\
 	  return "jmp %%pc@(2,%0:l)";			\
+	else if (TARGET_LONG_JUMP_TABLE_OFFSETS)	\
+	  return "jmp %%pc@(2,%0:l)";			\
 	else						\
 	  return "ext%.l %0\n\tjmp %%pc@(2,%0:l)";	\
       }							\
+    else if (TARGET_LONG_JUMP_TABLE_OFFSETS)		\
+      return "jmp %%pc@(2,%0:l)";			\
     else						\
       return "jmp %%pc@(2,%0:w)";			\
   } while (0)
--- gcc-7-20170101/gcc/config/m68k/m68k.h.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/config/m68k/m68k.h	2017-01-06 14:36:37.366245875 +0100
@@ -675,7 +675,7 @@  __transfer_from_trampoline ()					\
 
 /* This address is OK as it stands.  */
 #define PIC_CASE_VECTOR_ADDRESS(index) index
-#define CASE_VECTOR_MODE HImode
+#define CASE_VECTOR_MODE (TARGET_LONG_JUMP_TABLE_OFFSETS ? SImode : HImode)
 #define CASE_VECTOR_PC_RELATIVE 1
 
 #define DEFAULT_SIGNED_CHAR 1
@@ -857,7 +857,11 @@  do { if (cc_prev_status.flags & CC_IN_68
   asm_fprintf (FILE, "\t.long %LL%d\n", VALUE)
 
 #define ASM_OUTPUT_ADDR_DIFF_ELT(FILE, BODY, VALUE, REL)  \
-  asm_fprintf (FILE, "\t.word %LL%d-%LL%d\n", VALUE, REL)
+  asm_fprintf (FILE,						\
+	       TARGET_LONG_JUMP_TABLE_OFFSETS			\
+	       ? "\t.long %LL%d-%LL%d\n"			\
+	       : "\t.word %LL%d-%LL%d\n",			\
+	       VALUE, REL)
 
 /* We don't have a way to align to more than a two-byte boundary, so do the
    best we can and don't complain.  */
--- gcc-7-20170101/gcc/config/m68k/m68k.md.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/config/m68k/m68k.md	2017-01-06 14:36:37.366245875 +0100
@@ -6695,7 +6695,9 @@  (define_expand "tablejump"
 {
 #if CASE_VECTOR_PC_RELATIVE
     operands[0] = gen_rtx_PLUS (SImode, pc_rtx,
-				gen_rtx_SIGN_EXTEND (SImode, operands[0]));
+				TARGET_LONG_JUMP_TABLE_OFFSETS
+				? operands[0]
+				: gen_rtx_SIGN_EXTEND (SImode, operands[0]));
 #endif
 })
 
@@ -6710,12 +6712,36 @@  (define_insn "*tablejump_internal"
   [(set_attr "type" "jmp")])
 
 ;; Jump to variable address from dispatch table of relative addresses.
-(define_insn ""
+(define_insn "*tablejump_pcrel_si"
+  [(set (pc)
+	(plus:SI (pc)
+		 (match_operand:SI 0 "register_operand" "r")))
+   (use (label_ref (match_operand 1 "" "")))]
+  "TARGET_LONG_JUMP_TABLE_OFFSETS"
+{
+#ifdef ASM_RETURN_CASE_JUMP
+  ASM_RETURN_CASE_JUMP;
+#else
+  if (TARGET_COLDFIRE)
+    {
+      if (ADDRESS_REG_P (operands[0]))
+	return MOTOROLA ? "jmp (2,pc,%0.l)" : "jmp pc@(2,%0:l)";
+      else if (MOTOROLA)
+	return "jmp (2,pc,%0.l)";
+      else
+	return "jmp pc@(2,%0:l)";
+    }
+  else
+    return MOTOROLA ? "jmp (2,pc,%0.l)" : "jmp pc@(2,%0:l)";
+#endif
+})
+
+(define_insn "*tablejump_pcrel_hi"
   [(set (pc)
 	(plus:SI (pc)
 		 (sign_extend:SI (match_operand:HI 0 "register_operand" "r"))))
    (use (label_ref (match_operand 1 "" "")))]
-  ""
+  "!TARGET_LONG_JUMP_TABLE_OFFSETS"
 {
 #ifdef ASM_RETURN_CASE_JUMP
   ASM_RETURN_CASE_JUMP;
--- gcc-7-20170101/gcc/config/m68k/m68k.opt.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/config/m68k/m68k.opt	2017-01-06 14:45:19.305434346 +0100
@@ -142,6 +142,10 @@  mid-shared-library
 Target Report Mask(ID_SHARED_LIBRARY)
 Enable ID based shared library.
 
+mlong-jump-table-offsets
+Target Report RejectNegative Mask(LONG_JUMP_TABLE_OFFSETS)
+Use 32-bit offsets in jump tables rather than 16-bit offsets.
+
 mnobitfield
 Target RejectNegative InverseMask(BITFIELD)
 Do not use the bit-field instructions.
--- gcc-7-20170101/gcc/config/m68k/m68kelf.h.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/config/m68k/m68kelf.h	2017-01-06 14:36:37.366245875 +0100
@@ -58,9 +58,13 @@  along with GCC; see the file COPYING3.
       {							\
 	if (ADDRESS_REG_P (operands[0]))		\
 	  return "jmp %%pc@(2,%0:l)";			\
+	else if (TARGET_LONG_JUMP_TABLE_OFFSETS)	\
+	  return "jmp %%pc@(2,%0:l)";			\
 	else						\
 	  return "ext%.l %0\n\tjmp %%pc@(2,%0:l)";	\
       }							\
+    else if (TARGET_LONG_JUMP_TABLE_OFFSETS)		\
+      return "jmp %%pc@(2,%0:l)";			\
     else						\
       return "jmp %%pc@(2,%0:w)";			\
   } while (0)
--- gcc-7-20170101/gcc/config/m68k/netbsd-elf.h.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/config/m68k/netbsd-elf.h	2017-01-06 14:36:37.366245875 +0100
@@ -136,9 +136,13 @@  while (0)
       {							\
 	if (ADDRESS_REG_P (operands[0]))		\
 	  return "jmp %%pc@(2,%0:l)";			\
+	else if (TARGET_LONG_JUMP_TABLE_OFFSETS)	\
+	  return "jmp %%pc@(2,%0:l)";			\
 	else						\
 	  return "ext%.l %0\n\tjmp %%pc@(2,%0:l)";	\
       }							\
+    else if (TARGET_LONG_JUMP_TABLE_OFFSETS)		\
+      return "jmp %%pc@(2,%0:l)";			\
     else						\
       return "jmp %%pc@(2,%0:w)";			\
   } while (0)
--- gcc-7-20170101/gcc/doc/invoke.texi.~1~	2017-01-01 13:07:43.000000000 +0100
+++ gcc-7-20170101/gcc/doc/invoke.texi	2017-01-06 15:32:04.101803458 +0100
@@ -837,7 +837,7 @@  Objective-C and Objective-C++ Dialects}.
 -mno-short  -mhard-float  -m68881  -msoft-float  -mpcrel @gol
 -malign-int  -mstrict-align  -msep-data  -mno-sep-data @gol
 -mshared-library-id=n  -mid-shared-library  -mno-id-shared-library @gol
--mxgot -mno-xgot}
+-mxgot -mno-xgot -mlong-jump-table-offsets}
 
 @emph{MCore Options}
 @gccoptlist{-mhardlit  -mno-hardlit  -mdiv  -mno-div  -mrelax-immediates @gol
@@ -18460,6 +18460,11 @@  object file that accesses more than 8192
 These options have no effect unless GCC is generating
 position-independent code.
 
+@item -mlong-jump-table-offsets
+@opindex mlong-jump-table-offsets
+Use 32-bit offsets in @code{switch} tables.  The default is to use
+16-bit offsets.
+
 @end table
 
 @node MCore Options