This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] Add support for O32 FPXX ABI


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]