[PATCH 2/6] RISC-V gas port
Palmer Dabbelt
palmer@dabbelt.com
Wed Oct 19 18:43:00 GMT 2016
On Mon, 17 Oct 2016 05:09:06 PDT (-0700), nickc@redhat.com wrote:
>> We've just been running GCC tests. There is a RISC-V ISA test suite
>> that I can copy into the GAS port if you want, but I generally don't like
>> having copies of stuff floating around.
>
> Do the GCC tests just check the assembler output from the compiler, or do they
> also include checking the output of the assembler and disassembler ? If they do
> all three things then I agree that the GAS tests would be mostly redundant. But
> they do still serve a purpose. Or several really - they can be run without needing
> a compiler - they can test the generation of instructions that the compiler will
> not generate - they can test assembler features that the compiler does not use.
>
> I will not insist on an assembler testsuite being included as part of the
> contribution, but I do think that it is in your interest to have one. Perhaps as a
> later update.
As far as I know, the GCC test suite won't do a good job testing disassembly.
I think I'm going to hold off on committing a larger test suite until later.
Doing a good job here is going to be a lot of work, and I think doing a bad job
won't get us anything.
>> Our port was originally based on the MIPS port, but also contains a lot of code
>> from the Tilera and ARMv8 ports. Since we started by copying a lot of their
>> files we left the copyright headers intact. If that's not how it's supposed to
>> work then I can go change all the copyright dates, but my understand is that
>> these files are actually copyright FSF as of whatever their dates say.
>
> Fair enough - but please could you extend the dates to include 2016, since that
> is the year in which the sources will be added to the repository.
If there were any copyrights that don't include 2016 then I just missed them.
That's likely, as I miss these a lot.
>> https://github.com/riscv/riscv-binutils-gdb/commit/31454d5265bec2733075daf668b6ba70fd6e99ff
>>
>>>> + switch (elf_float_mode)
>>>> + {
>>>> + case FLOAT_MODE_DEFAULT:
>>>> + as_bad ("a specific float mode must be specified for an ELF");
>>>> + break;
>>>
>>> This error message looks a little wrong to me. I think that
>>> dropping the "for an ELF" part would make more sense.
>>
>> I think this is actually correct. The idea here is that ELF files in RISC-V
>> can be tagged with the floating-point ABI. This lets us avoid linking ELFs
>> with incompatible ABIs, much like 32-bit vs 64-bit ELFs. So in this case I
>> believe the "for an ELF" is necessary -- without it users might thing there's
>> something wrong with their GAS command-line arguments.
>
> OK, but "ELF" is just a binary file format - ie an abstract thing - and not a
> physical object like an actual file. Most users would, at least in my opinion,
> refer to binary files as "object files" or "executables" or "libraries", not
> "ELFs". So how about this alternative wording:
>
> a specific float mode must be specified for the output file
>
> or even:
>
> a specific float mode must be specified in order for the output file to be valid
>
> or just:
>
> please specify the float mode for the output
Good news: this error handling code has been removed and is replaced with a
more robust way to pass around the floating point ABIs.
> PS. I am going to wait for the v2 patches to be posted and then try reviewing
> en masse again.
Thanks! I'll try and get out a v2 today.
More information about the Binutils
mailing list