From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16159 invoked by alias); 8 Dec 2007 16:29:08 -0000 Received: (qmail 16150 invoked by uid 22791); 8 Dec 2007 16:29:06 -0000 X-Spam-Check-By: sourceware.org Received: from smtp.nildram.co.uk (HELO smtp.nildram.co.uk) (195.112.4.54) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 08 Dec 2007 16:29:00 +0000 Received: from firetop.home (85-211-134-127.dyn.gotadsl.co.uk [85.211.134.127]) by smtp.nildram.co.uk (Postfix) with ESMTP id 1A0672B63C2 for ; Sat, 8 Dec 2007 16:28:53 +0000 (GMT) Received: from richard by firetop.home with local (Exim 4.63) (envelope-from ) id 1J12Xw-00039B-DE for binutils@sourceware.org; Sat, 08 Dec 2007 16:28:56 +0000 From: Richard Sandiford To: binutils@sourceware.org Mail-Followup-To: binutils@sourceware.org, rsandifo@nildram.co.uk Subject: Fix code alignment for mixed mips16/nomips16 files Date: Sat, 08 Dec 2007 16:29:00 -0000 Message-ID: <87ir396th3.fsf@firetop.home> User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2007-12/txt/msg00039.txt.bz2 At the moment, ".align x" always fills with 0s on MIPS. This is a problem if, for example, you're writing MIPS16 assembly code in which a function has multiple entry points, and in which one entry point falls through to another. The natural thing would be to insert ".align 2" before each entry point, but 0 is not a nop in MIPS16 code. This patch adjusts mips_align's prototype so that the function can tell if the default fill pattern is being used. If so, it uses code alignment for code segments (just like the standard .align does) otherwise it continues to behave as it does now. I think all callers of mips_align besides s_align should be treated as wanting the default fill sequence rather than a zero fill sequence. (The choice is equivalent for non-code segments.) Also, mips_handle_align currently uses the final MIPS16 setting for all code fills. For .aligns, we should instead use whatever mode was in effect at the time the .align was assembled, and for end-of-file aligmments, we should use whatever mode was in effect when a .align or instruction was last assembled in that segment. The patch records the last .align or instruction ISA mode for a segment by adding a "mips16?" field to tc_segment_info_data. It allows a per-relaxation choice of ISA mode by defining NOP_OPCODE as a "mips16?" value. mips_handle_align can then check the first variable byte instead of mips_opts.mips16. Tested on mips64-linux-gnu. OK to install? Richard gas/ * config/tc-mips.h (mips_nop_opcode): Declare. (NOP_OPCODE): Define. (mips_segment_info): New structure. (TC_SEGMENT_INFO_TYPE): Use it instead of insn_label_list. * config/tc-mips.c (label_list): Adjust for new TC_SEGMENT_INFO_TYPE. (mips_record_mips16_mode): New function. (install_insn): Call it. (mips_align): Likewise. Turn the fill argument into an "int *". Use frag_align_code for code segments if no fill data is given. (s_align): Adjust call accordingly. (mips_nop_opcode): New function. (mips_handle_align): Use the first variable byte to decide which nop sequence is needed. Use md_number_to_chars and mips16_nop_insn. gas/testsuite/ * gas/mips/align2.s, gas/mips/align2.d, gas/mips/align2-el.d: New tests. * gas/mips/mips.exp: Run them. Index: gas/config/tc-mips.h =================================================================== --- gas/config/tc-mips.h 2007-12-08 14:44:58.000000000 +0000 +++ gas/config/tc-mips.h 2007-12-08 16:16:46.000000000 +0000 @@ -53,13 +53,20 @@ extern int mips_relax_frag (asection *, #define md_undefined_symbol(name) (0) #define md_operand(x) +extern char mips_nop_opcode (void); +#define NOP_OPCODE (mips_nop_opcode ()) + extern void mips_handle_align (struct frag *); #define HANDLE_ALIGN(fragp) mips_handle_align (fragp) #define MAX_MEM_FOR_RS_ALIGN_CODE (1 + 2) struct insn_label_list; -#define TC_SEGMENT_INFO_TYPE struct insn_label_list * +struct mips_segment_info { + struct insn_label_list *labels; + unsigned int mips16 : 1; +}; +#define TC_SEGMENT_INFO_TYPE struct mips_segment_info /* This field is nonzero if the symbol is the target of a MIPS16 jump. */ #define TC_SYMFIELD_TYPE int Index: gas/config/tc-mips.c =================================================================== --- gas/config/tc-mips.c 2007-12-08 14:44:58.000000000 +0000 +++ gas/config/tc-mips.c 2007-12-08 16:17:32.000000000 +0000 @@ -1185,7 +1185,7 @@ struct insn_label_list }; static struct insn_label_list *free_insn_labels; -#define label_list tc_segment_info_data +#define label_list tc_segment_info_data.labels static void mips_clear_insn_labels (void); @@ -1310,6 +1310,18 @@ create_insn (struct mips_cl_insn *insn, insn->mips16_absolute_jump_p = 0; } +/* Record the current MIPS16 mode in now_seg. */ + +static void +mips_record_mips16_mode (void) +{ + segment_info_type *si; + + si = seg_info (now_seg); + if (si->tc_segment_info_data.mips16 != mips_opts.mips16) + si->tc_segment_info_data.mips16 = mips_opts.mips16; +} + /* Install INSN at the location specified by its "frag" and "where" fields. */ static void @@ -1332,6 +1344,7 @@ install_insn (const struct mips_cl_insn } md_number_to_chars (f, insn->insn_opcode, 2); } + mips_record_mips16_mode (); } /* Move INSN to offset WHERE in FRAG. Adjust the fixups accordingly @@ -11994,14 +12007,22 @@ get_symbol (void) return p; } -/* Align the current frag to a given power of two. The MIPS assembler - also automatically adjusts any preceding label. */ +/* Align the current frag to a given power of two. If a particular + fill byte should be used, FILL points to an integer that contains + that byte, otherwise FILL is null. + + The MIPS assembler also automatically adjusts any preceding + label. */ static void -mips_align (int to, int fill, symbolS *label) +mips_align (int to, int *fill, symbolS *label) { mips_emit_delays (); - frag_align (to, fill, 0); + mips_record_mips16_mode (); + if (fill == NULL && subseg_text_p (now_seg)) + frag_align_code (to, 0); + else + frag_align (to, fill ? *fill : 0, 0); record_alignment (now_seg, to); if (label != NULL) { @@ -12017,8 +12038,7 @@ mips_align (int to, int fill, symbolS *l static void s_align (int x ATTRIBUTE_UNUSED) { - int temp; - long temp_fill; + int temp, fill_value, *fill_ptr; long max_alignment = 28; /* o Note that the assembler pulls down any immediately preceding label @@ -12040,17 +12060,18 @@ s_align (int x ATTRIBUTE_UNUSED) if (*input_line_pointer == ',') { ++input_line_pointer; - temp_fill = get_absolute_expression (); + fill_value = get_absolute_expression (); + fill_ptr = &fill_value; } else - temp_fill = 0; + fill_ptr = 0; if (temp) { segment_info_type *si = seg_info (now_seg); struct insn_label_list *l = si->label_list; /* Auto alignment should be switched on by next section change. */ auto_align = 1; - mips_align (temp, (int) temp_fill, l != NULL ? l->label : NULL); + mips_align (temp, fill_ptr, l != NULL ? l->label : NULL); } else { @@ -14321,36 +14342,40 @@ typedef struct proc { static procS *cur_proc_ptr; static int numprocs; -/* Fill in an rs_align_code fragment. */ +/* Implement NOP_OPCODE. We encode a MIPS16 nop as "1" and a normal + nop as "0". */ + +char +mips_nop_opcode (void) +{ + return seg_info (now_seg)->tc_segment_info_data.mips16; +} + +/* Fill in an rs_align_code fragment. This only needs to do something + for MIPS16 code, where 0 is not a nop. */ void mips_handle_align (fragS *fragp) { + char *p; + if (fragp->fr_type != rs_align_code) return; - if (mips_opts.mips16) + p = fragp->fr_literal + fragp->fr_fix; + if (*p) { - static const unsigned char be_nop[] = { 0x65, 0x00 }; - static const unsigned char le_nop[] = { 0x00, 0x65 }; - int bytes; - char *p; bytes = fragp->fr_next->fr_address - fragp->fr_address - fragp->fr_fix; - p = fragp->fr_literal + fragp->fr_fix; - if (bytes & 1) { *p++ = 0; fragp->fr_fix++; } - - memcpy (p, (target_big_endian ? be_nop : le_nop), 2); + md_number_to_chars (p, mips16_nop_insn.insn_opcode, 2); fragp->fr_var = 2; } - - /* For mips32, a nop is a zero, which we trivially get by doing nothing. */ } static void Index: gas/testsuite/gas/mips/align2.s =================================================================== --- /dev/null 2007-12-08 10:49:33.868054250 +0000 +++ gas/testsuite/gas/mips/align2.s 2007-12-08 15:18:45.000000000 +0000 @@ -0,0 +1,35 @@ + .text + .align 5 + .type f1,@function +f1: + .set mips16 + addiu $2,1 + .align 3 + addiu $3,1 + .size f1,.-f1 + + .align 2 + .set nomips16 + .type f2,@function +f2: + addiu $2,$2,1 + addiu $3,$3,1 + .align 4 + addiu $4,$4,1 + .align 3 + .size f2,.-f2 + + .set mips16 + .type f3,@function +f3: + addiu $16,$16,1 + .align 3 + .size f3,.-f3 + + .section .text.a,"ax",@progbits + .align 4 + .set nomips16 + .type f4,@function +f4: + addiu $5,$5,1 + .size f4,.-f4 Index: gas/testsuite/gas/mips/align2.d =================================================================== --- /dev/null 2007-12-08 10:49:33.868054250 +0000 +++ gas/testsuite/gas/mips/align2.d 2007-12-08 16:12:17.000000000 +0000 @@ -0,0 +1,41 @@ +# as: -EB +# objdump: -dr + +.* file format .* + +Disassembly of section \.text: + +00000000 : + 0: 4a01 addiu v0,1 + 2: 6500 nop + 4: 6500 nop + 6: 6500 nop + 8: 4b01 addiu v1,1 + a: 6500 nop + +0000000c : + c: 24420001 addiu v0,v0,1 + 10: 24630001 addiu v1,v1,1 + \.\.\. + 20: 24840001 addiu a0,a0,1 + 24: 00000000 nop + +00000028 : + 28: 4001 addiu s0,s0,1 + 2a: 6500 nop + 2c: 6500 nop + 2e: 6500 nop + 30: 6500 nop + 32: 6500 nop + 34: 6500 nop + 36: 6500 nop + 38: 6500 nop + 3a: 6500 nop + 3c: 6500 nop + 3e: 6500 nop + +Disassembly of section \.text\.a: + +00000000 : + 0: 24a50001 addiu a1,a1,1 + \.\.\. Index: gas/testsuite/gas/mips/align2-el.d =================================================================== --- /dev/null 2007-12-08 10:49:33.868054250 +0000 +++ gas/testsuite/gas/mips/align2-el.d 2007-12-08 16:12:32.000000000 +0000 @@ -0,0 +1,42 @@ +# source: align2.s +# as: -EL +# objdump: -dr + +.* file format .* + +Disassembly of section \.text: + +00000000 : + 0: 4a01 addiu v0,1 + 2: 6500 nop + 4: 6500 nop + 6: 6500 nop + 8: 4b01 addiu v1,1 + a: 6500 nop + +0000000c : + c: 24420001 addiu v0,v0,1 + 10: 24630001 addiu v1,v1,1 + \.\.\. + 20: 24840001 addiu a0,a0,1 + 24: 00000000 nop + +00000028 : + 28: 4001 addiu s0,s0,1 + 2a: 6500 nop + 2c: 6500 nop + 2e: 6500 nop + 30: 6500 nop + 32: 6500 nop + 34: 6500 nop + 36: 6500 nop + 38: 6500 nop + 3a: 6500 nop + 3c: 6500 nop + 3e: 6500 nop + +Disassembly of section \.text\.a: + +00000000 : + 0: 24a50001 addiu a1,a1,1 + \.\.\. Index: gas/testsuite/gas/mips/mips.exp =================================================================== --- gas/testsuite/gas/mips/mips.exp 2007-12-08 14:44:58.000000000 +0000 +++ gas/testsuite/gas/mips/mips.exp 2007-12-08 14:58:45.000000000 +0000 @@ -786,5 +786,7 @@ if { [istarget mips*-*-vxworks*] } { run_dump_test "noreorder" run_dump_test "align" + run_dump_test "align2" + run_dump_test "align2-el" run_dump_test "odd-float" }