Patchwork [MIPS] fix MIPS16 jump table overflow

login
register
mail settings
Submitter Andrew Pinski
Date Aug. 25, 2012, 9:31 p.m.
Message ID <CA+=Sn1mzaeT4qHwjAzkp09p4w2eR5=+414aHbynQqeAvGiLk9g@mail.gmail.com>
Download mbox | patch
Permalink /patch/179988/
State New
Headers show

Comments

Andrew Pinski - Aug. 25, 2012, 9:31 p.m.
On Sat, Aug 25, 2012 at 8:52 AM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> On 08/24/2012 11:46 PM, Richard Sandiford wrote:
>>
>> Andrew Pinski<pinskia@gmail.com>  writes:
>>>
>>> On Fri, Aug 24, 2012 at 10:08 PM, Andrew Pinski<pinskia@gmail.com>
>>> wrote:
>>>>
>>>> On Wed, Aug 22, 2012 at 7:15 PM, Sandra Loosemore
>>>> <sandra@codesourcery.com>  wrote:
>>>>>
>>>>> 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
>>>>
>>>>
>>>> Did you test this at all on a mips64-*-* target?  After this change
>>>> n64 jump tables are broken.
>>>> Before CASE_VECTOR_MODE was DImode for n64 .
>>>>
>>>> See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54371 for why it fails.
>>>> gpdword produces a double word for n64.
>>>>
>>>> For EABI64 it is ok to load 32bits because that the addresses are
>>>> 32bits but for n64, it is not ok.  The load addresses are normally
>>>> above the 32bits boundary.
>>>
>>>
>>> I am testing a patch which changes CASE_VECTOR_MODE to be:
>>> #define CASE_VECTOR_MODE ((mips_abi == ABI_64) ? DImode : SImode)
>>
>>
>> I think it should be:
>>
>> #define CASE_VECTOR_MODE \
>>    (TARGET_MIPS16_SHORT_JUMP_TABLES ? SImode : ptr_mode)
>>
>> #define CASE_VECTOR_SHORTEN_MODE(MIN, MAX, BODY) \
>>    (!TARGET_MIPS16_SHORT_JUMP_TABLES ? ptr_mode \
>>     : (MIN)>= -32768&&  (MAX)<  32768 : HImode \
>>
>>     : SImode)
>>
>> The point being that the TARGET_MIPS16_SHORT_JUMP_TABLES entries
>> are relative, so SImode would be correct there even for n64.
>>
>> I'd missed that CASE_VECTOR_MODE applied to tablejump as well
>> as casesi, sorry.
>
>
> Yeah, sorry.  :-(  The original version of the patch I posted (where
> CASE_VECTOR_MODE was #defined to ptr_mode) had indeed been tested on mips64,
> but not the revised version that was approved for check-in.


Here is the patch which I applied after a bootstrap/test on mips64-linux-gnu.

Thanks,
Andrew Pinski

ChangeLog:
* config/mips/mips.h (CASE_VECTOR_MODE): For not
TARGET_MIPS16_SHORT_JUMP_TABLES use ptr_mode.
(CASE_VECTOR_SHORTEN_MODE): Likewise.

Patch

Index: config/mips/mips.h
===================================================================
--- config/mips/mips.h	(revision 190638)
+++ config/mips/mips.h	(working copy)
@@ -2336,12 +2336,13 @@  typedef struct mips_args {
 
 #define JUMP_TABLES_IN_TEXT_SECTION TARGET_MIPS16_SHORT_JUMP_TABLES
 
-#define CASE_VECTOR_MODE SImode
+#define CASE_VECTOR_MODE (TARGET_MIPS16_SHORT_JUMP_TABLES ? SImode : 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 : SImode)
+  (!TARGET_MIPS16_SHORT_JUMP_TABLES ? ptr_mode \
+   : ((MIN) >= -32768 && (MAX) < 32768) ? HImode \
+   : SImode)
 
 #define CASE_VECTOR_PC_RELATIVE TARGET_MIPS16_SHORT_JUMP_TABLES