[PATCH][binutils][Arm][2/3] Add support for float16 (IEEE and Alternative formats) for Arm backend.

[Richard Earnshaw (lists)]

On 16/07/2019 02:59, Tamar Christina wrote:
> 
> Introducing a new directive for this seems counter intuitive to me. Particularly you open up some room for user error to happen more easily especially with inline assembly.

I expect the main usecase for this to be completely hand-written 
assembly.  GCC never outputs any .float type directives but translates 
the values it needs directly into the appropriate decimal constants for 
the encoded values.

> 
> You also then have to Orchestrate gcc to emit these as well in order for the file to be logically consistent with the command line options the user specified. If this is a directive gcc has to emit it as well.

No, see above.

> 
> Lastly it also duplicates information. Now the user has two directives to set when using fp16 in an arm assembly file and also opens up the weird possibility to have your built attribute be different from the "encoding type" directive which is a bit nonsensical.

No, it doesn't.  We can get gas to add the relevant build attributes 
when generating the output, if needed based on what directives the user 
actually put into their code (and whether or not they made use of the 
controlling option).  So no more duplication than we have for, say, .cpu 
or .arch.

> 
> Personally I would say If you're really keen on doing this that I'd much rather have either two float16 directives or for the float16 directive to take an optional "format" specifier rather than commandline options which would complicate support in gcc.
> 
> The fact that some platforms like PE don't support built attribute aren't an issue, as those platforms already wouldn't be able to switch formats and be stuck at the "default" one. In fact do we even support fp16 on these non-elf platforms.

It is an issue.  The attributes are stored in elf-specific extensions 
for the BFD.  They don't exist in COFF.  Furthermore, coff users simply 
don't expect to do this in their assembly code.

Finally, build attributes are pretty non-intuitive in their encoding. 
Expecting users to understand the value system is an insane idea.

R.

> 
> Regards,
> Tamar
> 
> ________________________________________
> From: binutils-owner@sourceware.org <binutils-owner@sourceware.org> on behalf of Richard Earnshaw (lists) <Richard.Earnshaw@arm.com>
> Sent: Monday, July 15, 2019 5:11:25 PM
> To: Barnaby Wilks; binutils@sourceware.org
> Cc: nd; nickc@redhat.com; Ramana Radhakrishnan
> Subject: Re: [PATCH][binutils][Arm][2/3] Add support for float16 (IEEE and Alternative formats) for Arm backend.
> 
> 
> 
> On 15/07/2019 16:44, Barnaby Wilks wrote:
>> Hi Richard,
>>
>> Yep agreed, a dedicated directive would make more sense really - originally I did have something
>> along those lines (.ieee_fp16_format/.alternative_fp16_format directives for switching between formats), but then
>> changed upon finding a EABI attribute for the same thing, and seeing that was how GCC did it.
>> Like you say though, for files that contain a mix of float16's in both IEEE and Alternative formats what would be the
>> EABI attribute for that be?
>>
>> This could be solved by just restricting to one format (in which
>> case a command line option may make more sense), or by inventing
>> a new EABI attribute? (I'm don't really know much about that, so have no idea if it's feasible).
>>
> 
> GCC already has the command-line option
> -mfp16-format=(none|ieee|alternative).  I'd suggest we have the same,
> and have an assembly attribute directive with a similar style.
> 
> For the directive, however, the default can be a little more lax: I
> think the assembler should start in a mode where the format is
> 'tentatively ieee' format.  Then, if a .float16 appears with no format
> override, we'd make that an explicit format selection.  At the end of
> assembly, when then generating build attributes (if doing that
> automatically, we'd update the build attributes if the format is at that
> point explicit.
> 
> The tentative nature is important to avoid changing behaviour for
> existing files: we don't want them to start writing out additional build
> attributes that they did not generate previously, however, ieee format
> is far more common than than alternative format, so requiring users to
> always specify this before ever using .float16 would be a little awkward.
> 
> R.
> 
>> Regards,
>>
>> Barney
>>
>> On 7/15/19 10:16 AM, Richard Earnshaw (lists) wrote:
>>>
>>>
>>> On 11/07/2019 16:26, Barnaby Wilks wrote:
>>>> Hello,
>>>>
>>>> This is part of a patch series that implements a directive for
>>>> assembling 16-bit floating point constants for Arm and AArch64.
>>>>
>>>> This patch implements the float16 directive for both the IEEE 754
>>>> format and the Arm alternative format for the
>>>> Arm backend >
>>>> The syntax of the directive is:
>>>> .float16 <0-n decimal numbers>
>>>> e.g.
>>>> .float16 12.0
>>>> .float16 0.23, 433.1, 0.06
>>>>
>>>> The Arm alternative format is almost identical to the IEEE 754
>>>> format, except that it doesn't
>>>> encode for NaNs or Infinity (instead an exponent of 0x1F represents a
>>>> normalized number in the range
>>>> 65536 to 131008).
>>>>
>>>> The alternative format is documented in the reference manual:
>>>> https://static.docs.arm.com/ddi0487/db/DDI0487D_b_armv8_arm.pdf?_ga=2.72318806.49764181.1561632697-999473562.1560847439
>>>>
>>>>
>>>> Which format is used is controlled by the
>>>> .eabi_attribute Tag_ABI_FP_16bit_format, <...>
>>>> directive, where if
>>>> <...> = 1 then use the IEEE 754 half-precision format else if
>>>> <...> = 2 then use the Arm alternative format
>>>>
>>>> Added testcases to verify the correct encoding for both formats (for
>>>> both big and little endian targets),
>>>> as well as tests verifying that warnings are thrown if trying to
>>>> encode NaN or Infinity when using
>>>> the Arm alternative format.
>>>> The encodings have been cross referenced with GCC's encoding for
>>>> __fp16 types to ensure consistency.
>>>>
>>>> Cross compiled and regtested on arm-none-eabi and
>>>> arm-none-linux-gnueabihf
>>>>
>>>> I don't have write access, so if it's OK then could someone commit on
>>>> my behalf?
>>>>
>>>
>>> Hi Barney,
>>>
>>> thanks for the patch.
>>>
>>> I'm not sure that using the build attributes to control selection of
>>> the format is the best idea.  Firstly, build attributes are not
>>> available when not assembling for ELF (we have PE-coff support in the
>>> assembler as well); secondly, build attributes should reflect what was
>>> placed in the assembly file, not control what goes into it.  So this
>>> is a case of the coach before the horse.
>>>
>>> I think, if we want to support the Arm alternate FP format in GAS at
>>> all, we need either a new command line option, or a new directive (or
>>> possibly both).  Then the code in BFD that does automatic
>>> build-attribute selection should be updated to reflect the tri-state
>>> situation: no format specified; IEEE format specified; or ARM
>>> Alternate format specified.
>>>
>>> Note that GAS can currently only reflect show build attributes that
>>> affect the whole file, so switching during assembly is a little
>>> dubious as things stand.
>>>
>>>
>>> R.
>>>
>>>
>>>> Thanks,
>>>> Barney
>>>>
>>>> gas/ChangeLog:
>>>>
>>>> 2019-07-11  Barnaby Wilks<barnaby.wilks@arm.com>
>>>>
>>>>       * config/tc-arm.c (md_atof): Set precision for float16 type.
>>>>       (enum fp_16bit_format): Add enum to represent the 2 float16
>>>> encodings.
>>>>       (arm_is_largest_exponent_ok): Check for whether to encode with
>>>> the IEEE or alternative
>>>>       format.
>>>>       * config/tc-arm.h (TC_LARGEST_EXPONENT_IS_NORMAL): Macro that
>>>> expands to
>>>>       arm_is_largest_exponent_ok.
>>>>       (arm_is_largest_exponent_ok): Add prototype for
>>>> arm_is_largest_exponent_ok function.
>>>>       * testsuite/gas/arm/float16-bad.d: New test.
>>>>       * testsuite/gas/arm/float16-bad.l: New test.
>>>>       * testsuite/gas/arm/float16-bad.s: New test.
>>>>       * testsuite/gas/arm/float16-be.d: New test.
>>>>       * testsuite/gas/arm/float16-be.s: New test.
>>>>       * testsuite/gas/arm/float16-le.d: New test.
>>>>       * testsuite/gas/arm/float16-le.s: New test.
>>>>