From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26820 invoked by alias); 23 Aug 2012 02:15:31 -0000 Received: (qmail 26761 invoked by uid 22791); 23 Aug 2012 02:15:27 -0000 X-SWARE-Spam-Status: No, hits=-3.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_MG X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 23 Aug 2012 02:15:13 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1T4Mwy-00025I-Ac from Sandra_Loosemore@mentor.com ; Wed, 22 Aug 2012 19:15:12 -0700 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 22 Aug 2012 19:15:12 -0700 Received: from [IPv6:::1] (147.34.91.1) by svr-orw-fem-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server id 14.1.289.1; Wed, 22 Aug 2012 19:15:11 -0700 Message-ID: <50359228.3070308@codesourcery.com> Date: Thu, 23 Aug 2012 02:15:00 -0000 From: Sandra Loosemore User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Lightning/1.0b2 Thunderbird/3.1.7 MIME-Version: 1.0 To: , Subject: Re: [PATCH, MIPS] fix MIPS16 jump table overflow References: <5032EAAA.2070703@codesourcery.com> <87boi456zv.fsf@talisman.home> In-Reply-To: <87boi456zv.fsf@talisman.home> Content-Type: multipart/mixed; boundary="------------090903020104080002070402" Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-08/txt/msg01540.txt.bz2 --------------090903020104080002070402 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2012 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] ) ] 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 Sandra Loosemore 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_): 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. --------------090903020104080002070402 Content-Type: text/x-patch; name="jumptable-new.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="jumptable-new.patch" Content-length: 7409 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_" + [(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 ("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 ("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) --------------090903020104080002070402--