diff mbox

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

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

Commit Message

Mikael Pettersson Jan. 6, 2017, 7:52 p.m. UTC 
Jeff Law writes:
 > 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

Indeed, good catch.  Revised patch below.   /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, 9:21 p.m. UTC | #1 
On 01/06/2017 12:52 PM, Mikael Pettersson wrote:
> 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.
Committed.  Thanks!

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 20:16:25.278132607 +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 20:16:25.278132607 +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 20:21:32.677641812 +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,26 @@  (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
+  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 20:16:25.278132607 +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 20:16:25.278132607 +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 20:16:25.278132607 +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 20:16:25.278132607 +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