This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [PATCHv3] Add support for O32 FPXX ABI
- From: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: "macro at codesourcery dot com" <macro at codesourcery 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, 27 May 2014 14:58:39 +0000
- Subject: RE: [PATCHv3] Add support for O32 FPXX ABI
- Authentication-results: sourceware.org; auth=none
- References: <6D39441BF12EF246A7ABCE6654B0235353ADA0 at LEMAIL01 dot le dot imgtec dot org> <87bnumf4us dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B0235353F99C at LEMAIL01 dot le dot imgtec dot org> <87mwe31el7 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com>
Richard Sandiford <rdsandiford@googlemail.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> >> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> >> Not sure we should run check_fpabi on the implicitly-chosen ABI,
> >> since it would produce messages about .gnu_attribute when the user
> >> hasn't used it. Maybe:
> >>
> >> if (obj_elf_seen_attribute (OBJ_ATTR_GNU, Tag_GNU_MIPS_ABI_FP))
> >> {
> >> /* Perform consistency checks on the floating-point ABI. */
> >> fpabi = bfd_elf_get_obj_attr_int (stdoutput, OBJ_ATTR_GNU,
> >> Tag_GNU_MIPS_ABI_FP);
> >> if (fpabi != Val_GNU_MIPS_ABI_FP_ANY)
> >> check_fpabi (fpabi);
> >> }
> >> else
> >> {
> >> ...
> >> }
> >
> > I have no major objection to this. I had structured it the way it was
> > to catch internal errors if the ABI inferring code was inconsistent
> > with the ABI checks. If the internals were correct then there should
> > be no warnings raised from inferred FP ABIs.
>
> But the point was that it would be flagged up as a user error rather
> than an internal error. Either we should raise it as an internal error
> or we should go with the above.
True. I've restructured this as above. Anyone changing this code will almost
certainly have had to understand all this in great detail and will be asked
to add test cases for any extensions so internal errors should get caught.
> >> The rest looks OK to me too. TBH I've read through versions of this
> >> patch so many times that I'm probably not going to pick up anything
> >> useful (compared to someone coming to it fresh).
> >
> > I did have a thought on the ABI flags merging/checking logic. I have
> > essentially ignored the content of flags1 an flags2 currently as they
> > are entirely undefined at the moment. Do you think I should assert
> > them to be zero, allow them to be carried forward into the output if
> > the inputs match, or do nothing as now? I'm tempted to check that they
> > are zero.
>
> I don't really mind. Maybe we could define flags1 as being an ORed
> field
> and leave flags2 as something that can only be processed if you know
> what it means (error on nonzero, like you say)?
Good call. May as well make use of having two fields.
I'll post the final patch here for the archive when it gets committed.
Regards,
Matthew