This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: QNX MIPS Support Patch
Jeff Baker wrote:
[snip]
> >>qnx_shared_textrel.patch - This patch adds the --warn-shared-textrel
> >>option to the linker. This option, oddly enough, warns when a
> >>DT_TEXTREL is being added to a shared object.
> >
> >I can't approve this one.
>
> Can't because it's not your area or can't because it's no good?
It looks good IMHO, but it is outside the mips area.
[snip]
> - A patch to add support for QNX MIPS PIC to the BFD.
>
> What is the accepted way of switching on our modifications without using
> the ifdefs (they were never meant to be permanent, I just don't know how
> else to do it yet)?
The object files should have some identification as being QNX PIC
specific (probably a .note section?), and BFD should switch
behaviour accordingly.
[snip]
> 2004-08-26 Jeff Baker <jbaker@qnx.com>
>
> * bfd/libbfd.h: Add new reloc: R_MIPS_QNX_COPY.
> * bfd/elf32-mips.c: Likewise.
> * bfd/bfd-in2.h: Likewise.
> * bfd/reloc.c: Likewise.
> * include/elf/mips.h: Likewise.
Ok, but note that bfd/ChangeLog and include/elf/ChangeLog are in
different subdirectories. When you commit, add the entry to the nearest
ChangeLog in the tree, with proper relative paths. E.g. for bfd:
2004-08-26 Jeff Baker <jbaker@qnx.com>
* libbfd.h: Add new reloc: R_MIPS_QNX_COPY.
* elf32-mips.c: Likewise.
* bfd-in2.h: Likewise.
* reloc.c: Likewise.
[snip]
> 2004-08-26 Jeff Baker <jbaker@qnx.com>
> * config/tc-mips.c (QNX_GP_REG): Define.
> (load_address): Added code to handle QNX_PIC pic level.
> (macro): Likewise.
> (mips_validate_fix): Likewise.
> (mips_fix_adjustable): Likewise.
> (s_cpsetup): Likewise.
> (s_cplocal): Likewise.
> (s_cprestore): Likewise.
> (s_cpreturn): Likewise.
> (s_cpadd): Likewise.
> (s_gpvalue): Likewise.
> (s_gpword): Likewise.
> (s_gpdword): Likewise.
> (md_longopts): Define qnx_pic command line argument.
> (md_parse_option): Handle qnx_pic command line argument.
> (s_qnxpiccalls): New function.
> (s_noqnxpiccalls): New function.
> * config/tc-mips.h (mips_pic_level): Add QNX_PIC pic
> level.
[snip]
> @@ -3877,21 +3882,21 @@ load_address (int reg, expressionS *ep,
> mips_gp_register, BFD_RELOC_GPREL16);
> relax_switch ();
> }
> macro_build_lui (ep, reg);
> macro_build (ep, ADDRESS_ADDI_INSN, "t,r,j",
> reg, reg, BFD_RELOC_LO16);
> if (mips_relax.sequence)
> relax_end ();
> }
> }
> - else if (mips_pic == SVR4_PIC && ! mips_big_got)
> + else if ((mips_pic == SVR4_PIC || mips_pic == QNX_PIC) && ! mips_big_got)
A common mips_pic_p macro would be nicer.
[snip]
> @@ -5062,21 +5067,21 @@ macro (struct mips_cl_insn *ip)
> load_delay_nop ();
> macro_build (NULL, ADDRESS_ADD_INSN, "d,v,t",
> treg, AT, breg);
> breg = 0;
> tempreg = treg;
> }
> add_got_offset_hilo (tempreg, &offset_expr, AT);
> used_at = 1;
> }
> }
> - else if (mips_pic == SVR4_PIC && ! mips_big_got && HAVE_NEWABI)
> + else if ((mips_pic == SVR4_PIC || mips_pic == QNX_PIC) && ! mips_big_got && HAVE_NEWABI)
Is there some plan to implement a NewABI variant as well? If yes,
add at least a TODO comment to the relevant s_cp*/s_gp* functions.
If no, don't check for QNX_PIC in NewABI code.
[snip]
> @@ -5467,61 +5472,64 @@ macro (struct mips_cl_insn *ip)
>
> /* The jal instructions must be handled as macros because when
> generating PIC code they expand to multi-instruction
> sequences. Normally they are simple instructions. */
> case M_JAL_1:
> dreg = RA;
> /* Fall through. */
> case M_JAL_2:
> if (mips_pic == NO_PIC)
> macro_build (NULL, "jalr", "d,s", dreg, sreg);
> - else if (mips_pic == SVR4_PIC)
> + else if (mips_pic == SVR4_PIC || mips_pic == QNX_PIC)
> {
> - if (sreg != PIC_CALL_REG)
> + if ((sreg != PIC_CALL_REG) && (mips_pic != QNX_PIC))
> as_warn (_("MIPS PIC call to register other than $25"));
Does QNX PIC use different registers for calls?
[snip]
> @@ -5578,20 +5586,28 @@ macro (struct mips_cl_insn *ip)
> relax_end ();
> }
>
> macro_build_jalr (&offset_expr);
> }
> else
> {
> relax_start (offset_expr.X_add_symbol);
> if (! mips_big_got)
> {
> + if(mips_pic != QNX_PIC)
> + macro_build (&offset_expr,
> + ((bfd_arch_bits_per_address (stdoutput) == 32
> + || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
> + ? "lw" : "ld"),
> + "t,o(b)", PIC_CALL_REG,
> + (int) BFD_RELOC_MIPS_CALL16, mips_gp_register);
> + else
> macro_build (&offset_expr, ADDRESS_LOAD_INSN, "t,o(b)",
> PIC_CALL_REG, BFD_RELOC_MIPS_CALL16,
> mips_gp_register);
This chunk is apparently some old code, and completely superfluous.
[snip]
> @@ -10120,20 +10138,22 @@ struct option md_longopts[] =
> #define OPTION_64 (OPTION_ELF_BASE + 6)
> {"64", no_argument, NULL, OPTION_64},
> #define OPTION_MDEBUG (OPTION_ELF_BASE + 7)
> {"mdebug", no_argument, NULL, OPTION_MDEBUG},
> #define OPTION_NO_MDEBUG (OPTION_ELF_BASE + 8)
> {"no-mdebug", no_argument, NULL, OPTION_NO_MDEBUG},
> #define OPTION_PDR (OPTION_ELF_BASE + 9)
> {"mpdr", no_argument, NULL, OPTION_PDR},
> #define OPTION_NO_PDR (OPTION_ELF_BASE + 10)
> {"mno-pdr", no_argument, NULL, OPTION_NO_PDR},
> +#define OPTION_QNX_PIC (OPTION_ELF_BASE + 11)
> + {"qnx_pic", no_argument, NULL, OPTION_QNX_PIC},
> #endif /* OBJ_ELF */
[snip]
> + case OPTION_QNX_PIC:
> + if (OUTPUT_FLAVOR != bfd_target_elf_flavour)
> + {
> + as_bad ("-qnx_pic is supported only for ELF format");
> + return 0;
> + }
I would prefer more gcc-like options, with an inverse, and probably
without underscore. E.g:
-mqnx-pic
-mno-qnx-pic
If that causes a problem with backward compatibility, -qnx_pic could be
retained as alias for -mqnx-pic.
[snip]
> /* Handle the .cpload pseudo-op. This is used when generating SVR4
> PIC code. It sets the $gp register for the function based on the
> function address, which is in the register named in the argument.
> This uses a relocation against _gp_disp, which is handled specially
> by the linker. The result is:
> lui $gp,%hi(_gp_disp)
> addiu $gp,$gp,%lo(_gp_disp)
> addu $gp,$gp,.cpload argument
> - The .cpload argument is normally $25 == $t9. */
> + The .cpload argument is normally $25 == $t9.
> +
> + In the case of QNX pic code, we always compute the full address
> + of the GOT, therefore we do:
> + bltzal 0, 0f
> + nop
> + 0: lui $23, %hi(_gp_disp)
> + addiu $23, $23, %lo(_gp_disp)
> + addu $23, $23, $31
> + One possible optimization would be to move the opcode
> + right before the bltzal into the nop slot. */
Below, this optimization is done, so the comment should reflect that.
Out of curiosity: Why isn't simply the lui moved into the branch delay
slot? At least it would always be a valid optimization.
> static void
> s_cpload (int ignore ATTRIBUTE_UNUSED)
> {
> expressionS ex;
>
> /* If we are not generating SVR4 PIC code, or if this is NewABI code,
> .cpload is ignored. */
> - if (mips_pic != SVR4_PIC || HAVE_NEWABI)
> + if ((mips_pic != SVR4_PIC && mips_pic != QNX_PIC) || HAVE_NEWABI)
> {
> s_ignore (0);
> return;
> }
>
> /* .cpload should be in a .set noreorder section. */
> - if (mips_opts.noreorder == 0)
> + if ((mips_opts.noreorder == 0) && (mips_pic != QNX_PIC))
> as_warn (_(".cpload not in noreorder section"));
>
> ex.X_op = O_symbol;
> ex.X_add_symbol = symbol_find_or_make ("_gp_disp");
> ex.X_op_symbol = NULL;
> ex.X_add_number = 0;
>
> /* In ELF, this symbol is implicitly an STT_OBJECT symbol. */
> symbol_get_bfdsym (ex.X_add_symbol)->flags |= BSF_OBJECT;
>
> + if (mips_pic == QNX_PIC) {
> + expressionS ep;
> + /* In our case, we do a bltzal first to get the current IP, and
In MIPS lingua, it is "PC", not "IP". :-)
> + * then add the offset to the got... Thus, we always do "cpload $31"
> + */
> + ep.X_op = O_constant;
> + ep.X_add_number = 4;
> + tc_get_register(0); // Flush any arg to cpload...
No C99 comments, please.
> + /* See if we can swap the bltzal with the previous insn and save
> + * the nop...
> + * For simplicity, we only check that the insn is a move or sw,
> + * which are really the only two we should encounter.
> + * If it is anything else, we don't swap for now.
> + */
> + if(0 && (mips_optimize > 1)
> + && prev_insn_valid
> + && (mips_opts.isa > 1)
> + && !(mips_opts.noreorder)
> + && (((prev_insn.insn_opcode & 0xfc1f07ff) == 0x00000021)
> + || ((prev_insn.insn_opcode & 0xfc000000) == 0xac000000)))
> + {
> + char *prev_f;
> + char temp[4];
> +
> + prev_f = prev_insn_frag->fr_literal + prev_insn_where;
> + memcpy (temp, prev_f, 4);
> + *(unsigned long *)(prev_f) = 0x04100001;
> + macro_build (&ep,
> + "bltzal", "s,p", 0);
Formatting, ...
> + prev_f = prev_insn_frag->fr_literal + prev_insn_where;
> + memcpy (prev_f, temp, 4);
> + mips_opts.noreorder ++;
^
..., formatting, ...
> + }
> + else
> + { /* We need a NOP... */
> + mips_opts.noreorder ++;
> + macro_build (&ep,
> + "bltzal", "s,p", 0);
> + macro_build ((expressionS *) NULL, "nop", "");
..., old K&R casts, ...
> + }
> + macro_build_lui (&ex, 23);
> + macro_build (&ex, "addiu", "t,r,j", 23, 23,
> + (int) BFD_RELOC_LO16);
..., and magic constants instead of QNX_GP_REG aka mips_gp_register, ...
> + mips_opts.noreorder --;
> +
> + macro_build ((expressionS *) NULL, "addu", "d,v,t",
> + 23, 23, RA);
> + } else
> + {
> +
> macro_start ();
> macro_build_lui (&ex, mips_gp_register);
> macro_build (&ex, "addiu", "t,r,j", mips_gp_register,
> mips_gp_register, BFD_RELOC_LO16);
> macro_build (NULL, "addu", "d,v,t", mips_gp_register,
> mips_gp_register, tc_get_register (0));
> macro_end ();
> +}
... and a suspiciously absent macro_start()/macro_end() pair.
[snip]
> @@ -12743,20 +12869,28 @@ mips_fix_adjustable (fixS *fixp)
> if (fixp->fx_r_type == BFD_RELOC_MIPS16_JMP)
> return 0;
>
> if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
> || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
> return 0;
>
> if (fixp->fx_addsy == NULL)
> return 1;
>
> + if ((mips_pic == QNX_PIC) &&
> + (fixp->fx_r_type == BFD_RELOC_MIPS_GOT16 || fixp->fx_r_type == BFD_RELOC_32))
Line length formatting.
[snip]
> 2004-08-26 Jeff Baker <jbaker@qnx.com>
> * ltconfig (nto-qnx*): Add support for QNX OS suffix.
> * bfd/config.bfd (mips*eb-*-nto*): New target.
> (mips*-*-nto*): New target.
> * gas/configure (mips*-*-nto*): New target.
> * gas/configure.in: Likewise.
> * ld/Makefile.am: Add targets for QNX MIPS emulation files.
> * ld/Makefile.in: Likewise.
> * ld/configure.tgt(mips*-*-nto*): New target.
> * ld/emulparams/elf32bmipnto.sh: New file. QNX MIPS
> emulation.
> * ld/emulparams/elf32lmipnto.sh: New file. QNX MIPS
> emulation.
This also needs to be split in the respective subdirectory's ChangeLogs.
[snip]
> --- bfd/config.bfd 19 Aug 2004 18:09:44 -0000 1.169
> +++ bfd/config.bfd 26 Aug 2004 19:45:29 -0000
> @@ -829,20 +829,30 @@ case "${targ}" in
> targ_selvecs=ecoff_little_vec
> ;;
> mips*el-*-elf* | mips*el-*-rtems* | mips*el-*-vxworks* | mips*-*-chorus*)
> targ_defvec=bfd_elf32_littlemips_vec
> targ_selvecs="bfd_elf32_bigmips_vec bfd_elf64_bigmips_vec bfd_elf64_littlemips_vec"
> ;;
> mips*-*-elf* | mips*-*-rtems* | mips*-*-vxworks | mips*-*-windiss)
> targ_defvec=bfd_elf32_bigmips_vec
> targ_selvecs="bfd_elf32_littlemips_vec bfd_elf64_bigmips_vec bfd_elf64_littlemips_vec"
> ;;
> + mips*eb-*-nto*)
> + targ_defvec=bfd_elf32_bigmips_vec
> + targ_selvecs="bfd_elf32_littlemips_vec"
> + targ_cflags=-D__QNXTARGET__
> + ;;
> + mips*-*-nto*)
> + targ_defvec=bfd_elf32_littlemips_vec
> + targ_selvecs="bfd_elf32_bigmips_vec"
> + targ_cflags=-D__QNXTARGET__
> + ;;
Please don't define __QNXTARGET__. :-)
[snip]
> Index: ld/configure.tgt
> ===================================================================
> RCS file: /cvs/src/src/ld/configure.tgt,v
> retrieving revision 1.154
> diff -w -u -1 -0 -p -r1.154 configure.tgt
> --- ld/configure.tgt 19 Aug 2004 18:11:42 -0000 1.154
> +++ ld/configure.tgt 26 Aug 2004 19:45:35 -0000
> @@ -421,20 +421,23 @@ mips*-*-netbsd*) targ_emul=elf32bmip
> ;;
> mips*-*-bsd*) targ_emul=mipsbig ;;
> mips*vr4300el-*-elf*) targ_emul=elf32l4300 ;;
> mips*vr4300-*-elf*) targ_emul=elf32b4300 ;;
> mips*vr4100el-*-elf*) targ_emul=elf32l4300 ;;
> mips*vr4100-*-elf*) targ_emul=elf32b4300 ;;
> mips*vr5000el-*-elf*) targ_emul=elf32l4300 ;;
> mips*vr5000-*-elf*) targ_emul=elf32b4300 ;;
> mips*el-*-elf*) targ_emul=elf32elmip ;;
> mips*-*-elf*) targ_emul=elf32ebmip ;;
> +mips*-*-nto*) targ_emul=elf32lmipnto
> + targ_extra_emuls="elf32bmipnto"
> + ;;
I think this wants an additional mips*eb*-*-nto* with the big endian
emulation as default.
Thiemo