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

[Richard Sandiford <rdsandiford at googlemail dot com>]

Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
>> > 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.
>
> I don't have a big problem with fpr_size but the only thing we can use it
> for is to track the widest required register. I implemented gpr_size to be
> stricter as we can't mix 32-bit and 64-bit ABIs though I guess just
> tracking widest would be OK there as well?

Yeah, it probably amounts to the same thing.  With EABI32 and EABI64 being
treated as separate ABI enums, the ABI check (which should happen first)
would catch mismatches.

Again, the only case I can think of where it could matter would be
for 128-bit GPRs, which could be used with existing ABIs thanks to
the way that the upper 64 bits weren't touched by the 32-bit and
64-bit load and arithmetic insns (IIRC, obviously this is going
back a few years).  In that case taking the widest would be correct.

> I think I've realised what scenario you are accounting for with MSA.
> Something like... 'MSAv2' that supports more vector operations and also
> 256-bit registers but it is possible to use just the 128-bit subset and
> make use of the new operations.

Right.

> I'd propose changing the start of the structure to the following:
>
> unsigned short version;
> unsigned char isa_level; 
> unsigned char isa_rev; 
> unsigned char gpr_size; 
> unsigned char cpr1_size; 
> unsigned char cpr2_size;
> unsigned char fp_abi;

Sounds good to me, thanks.

>> 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

Realised later that this wasn't strictly true.  When one ISA extends
another, those two can be merged and take the maximum.

>> 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).
>
> I realised this after starting to change things.  I'd like to find a way to
> use the same defines in the structure and gas internals if possible even if
> one has to be derived rather than direct. I.e. construct a bit mask with
> 1 << AFL_EXT_something.

I agree using the same enum internally would be good if it's natural
and easy.  And maybe it would be.  The opcodes table uses an abbreviated
form of the INSN_* flags anyway.  But if it gets too convoluted then
having separate internal numbering would be fine too.

Thanks,
Richard