This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 1/2] Add support for O32 FPXX ABI
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: Matthew Fortune <Matthew dot Fortune at imgtec dot com>, "Joseph Myers (joseph at codesourcery dot com)" <joseph at codesourcery dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>, Rich Fuhler <Rich dot Fuhler at imgtec dot com>
- Date: Tue, 6 May 2014 23:47:00 +0100
- Subject: Re: [PATCH 1/2] Add support for O32 FPXX ABI
- Authentication-results: sourceware.org; auth=none
- References: <6D39441BF12EF246A7ABCE6654B023534DAAAB at LEMAIL01 dot le dot imgtec dot org> <87fvll1ha9 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <6D39441BF12EF246A7ABCE6654B023534E07CB at LEMAIL01 dot le dot imgtec dot org> <87k3awnuf1 dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B023534E0F50 at LEMAIL01 dot le dot imgtec dot org> <87ppknzv7m dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B023534E16D7 at LEMAIL01 dot le dot imgtec dot org> <877g6vzpj1 dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B023534E3030 at LEMAIL01 dot le dot imgtec dot org> <87eh0yl4xx dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B023534E3AA8 at LEMAIL01 dot le dot imgtec dot org> <87d2ghk7fu dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B023535127C8 at LEMAIL01 dot le dot imgtec dot org> <87r44mve6s dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <6D39441BF12EF246A7ABCE6654B02353512A10 at LEMAIL01 dot le dot imgtec dot org> <87r44leu34 dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B0235351D76A at LEMAIL01 dot le dot imgtec dot org> <87r446ww90 dot fsf at talisman dot default>
On Tue, 6 May 2014, Richard Sandiford wrote:
> > +/* Return true if the given ELF header flags describe a 32-bit binary. */
> > +
> > +static bfd_boolean
> > +mips_32bit_flags_p (flagword flags)
> > +{
> > + return ((flags & EF_MIPS_32BITMODE) != 0
> > + || (flags & EF_MIPS_ABI) == E_MIPS_ABI_O32
> > + || (flags & EF_MIPS_ABI) == E_MIPS_ABI_EABI32
> > + || (flags & EF_MIPS_ARCH) == E_MIPS_ARCH_1
> > + || (flags & EF_MIPS_ARCH) == E_MIPS_ARCH_2
> > + || (flags & EF_MIPS_ARCH) == E_MIPS_ARCH_32
> > + || (flags & EF_MIPS_ARCH) == E_MIPS_ARCH_32R2);
> > +}
>
> OK, this is preexisting code, but I'm not sure it's right.
> 32bitness should be taken purely from the ABI, not the architecture,
> otherwise we could treat 32-bit MIPS64r2 code differently from MIPS32r2
> code.
>
> Since o32 was the original (with no flags), I think we should have
> mips_64bit_flags_p instead.
FWIW, it was my first thought too upon spotting this addition, before I
realised it's really not one, and this change merely moves the function
around intact. Upon further checking this code seems to try mimicking
what GAS does in mips_after_parse_args:
/* This flag is set when we have a 64-bit capable CPU but use only
32-bit wide registers. Note that EABI does not use it. */
if (ISA_HAS_64BIT_REGS (mips_opts.isa)
&& ((mips_abi == NO_ABI && file_mips_gp32 == 1)
|| mips_abi == O32_ABI))
mips_32bitmode = 1;
and is really very old. Note that in this case where ISA_HAS_64BIT_REGS
and `mips_abi == NO_ABI' are both true (the latter condition being a rare
corner case, possible for ELF and I think IRIX 5 toolchains only where
MIPS_DEFAULT_ABI is NO_ABI) GAS will set EF_MIPS_32BITMODE according to
-mgp32/-mgp64 and the ABI bits will remain unset.
E_MIPS_ABI_O32 is also omitted entirely on IRIX, according to
USE_E_MIPS_ABI_O32, so we may want to consider defining ABI_O32_P like
below instead:
/* Nonzero if ABFD is using the O32 ABI. */
#define ABI_O32_P(abfd) \
((elf_elfheader (abfd)->e_flags & EF_MIPS_ABI) == E_MIPS_ABI_O32
|| (!ABI_64_P (abfd)
&& (elf_elfheader (abfd)->e_flags & (EF_MIPS_ABI | EF_MIPS_ABI2)) == 0))
though I'm afraid the answer from this predicate won't be accurate either,
for a different set of cases.
> > @@ -14688,6 +15083,86 @@ _bfd_mips_elf_merge_private_bfd_data (bfd *ibfd, bfd *obfd)
> > if (null_input_bfd)
> > return TRUE;
> >
> > + /* Populate abiflags using existing information. */
> > + if (!mips_elf_tdata (ibfd)->abiflags_valid)
> > + {
> > + infer_mips_abiflags (ibfd, &mips_elf_tdata (ibfd)->abiflags);
> > + mips_elf_tdata (ibfd)->abiflags_valid = TRUE;
> > + }
> > + else
> > + {
> > + Elf_Internal_ABIFlags_v0 abiflags;
> > + Elf_Internal_ABIFlags_v0 *iabiflags;
> > + infer_mips_abiflags (ibfd, &abiflags);
> > + iabiflags = &mips_elf_tdata (ibfd)->abiflags;
> > +
> > + if (iabiflags->isa_level != abiflags.isa_level
> > + || iabiflags->isa_rev != abiflags.isa_rev)
> > + (*_bfd_error_handler)
> > + (_("%B: warning: Inconsistent ISA between e_flags and "
> > + ".MIPS.abiflags"), ibfd);
> > + if (abiflags.fp_abi != Val_GNU_MIPS_ABI_FP_ANY
> > + && iabiflags->fp_abi != abiflags.fp_abi)
> > + (*_bfd_error_handler)
> > + (_("%B: warning: Inconsistent FP ABI between e_flags and "
> > + ".MIPS.abiflags"), ibfd);
> > + if ((iabiflags->ases & abiflags.ases) != abiflags.ases)
> > + (*_bfd_error_handler)
> > + (_("%B: warning: Inconsistent ASEs between e_flags and "
> > + ".MIPS.abiflags"), ibfd);
>
> Minor nit, but "abiflags" vs. "iabiflags" is a bit hard to follow,
> since the "i" could be "implicit" (but I assume is "input").
`in_abiflags' perhaps? -- following `in_attr' and `out_attr' from
mips_elf_merge_obj_attributes.
Maciej