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: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Matthew Fortune <Matthew dot Fortune at imgtec 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: Thu, 24 Apr 2014 18:18:35 +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> <87a9c0xkje dot fsf at talisman dot default> <6D39441BF12EF246A7ABCE6654B023534DC37D at LEMAIL01 dot le dot imgtec dot org> <87ha655s3i dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <6D39441BF12EF246A7ABCE6654B023534DC8CF at LEMAIL01 dot le dot imgtec dot org> <871tx9z35k dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <6D39441BF12EF246A7ABCE6654B023534DFC40 at LEMAIL01 dot le dot imgtec dot org> <878urd37t7 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <6D39441BF12EF246A7ABCE6654B023534DFD9B 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>
Thanks for the patch. I'll try to review it in detail over the weekend,
but just some quick comments about the ABI:
Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> 1) I'm not sure there is much value encoding the gpr_size as log2
> versus just having an enum for the two current widths. I'm not that
> bothered though so if you have a gut feeling it would be better log2
> then fine.
An enum is fine with me. The only other obvious value would be 128
(r5900/r7900), which would presumably be enum value 3, so we end up
with log2 - 4 anyway.
> 2) I changed fpr_size to fp_abi. It gives some synergy with
> gnu_attributes and I have re-used the same values
OK, but...
> 3) I'm not sure we need to track FP register width specifically. In
> the end the MSA ABI is actually a no-change so recording MSA ABI is
> not necessary but recording the presence of the MSA ASE is useful. I'm
> suggesting we use this along with the fp_abi to determine the width of
> the fp registers.
...the reason I suggested an fpr size was that it allowed the MSA
register width to be separated from the ASE revision number.
Let's specify this on the assumption that there'll be a 256-bit MSA
in future. What would be the best way of representing that? I'd rather
not use the ASE field, since I think ASE compatibility should as far
as possible be a case of ORing the input ASEs together.
> 4) isa_ext and ases make direct use of the existing masks used by gas
> and I have moved all these to be part of the include/elf/mips.h
> header. This does leave one small annoyance which is that 64-bit
> versions of ASEs don't really need to be included in this mask but are
> for reasons internal to gas. I'm not sure what to do with this, I'm
> thinking some rework of tc-mips to avoid the need for the 64-bit
> versions of the ASEs in the mask.
I'll try to look at this over the weekend. I agree we don't want to
expose the 64-bit flags outside binutils.
> As before it would be useful to get comments on the ABI aspects of
> this first. I will then update the FPXX specification accordingly when
> the last few details are ironed out.
My main concern is about the isa_ext field being a bitmask rather than
an enum. The reason we use a bitmask internally is because some instructions
are defined in the same way for more than one processor extension and using
a bitmask avoids duplicate opcode entries. But I think the isa_ext field
is conceptually an enum and compatibility should just be a case of:
0 comb X -> X
X comb 0 -> X
X comb X -> X
error otherwise
AFAICT, for all current values, any isa_ext with two bits set would
either be impossible (because the two extensions aren't compatible) or
be redundant (because one extension is itself an extension of the other).
FWIW, the ABI side looks good to me otherwise at first glance.
Thanks,
Richard