[00/11] gas: adjustments to floating point data directives handling

Message ID 51adf2ca-3dbf-f2ae-ebe9-68dfee54c759@suse.com
Headers show
Series
  • gas: adjustments to floating point data directives handling
Related show

Message

Alan Modra via Binutils July 23, 2021, 6:49 a.m.
This started out with me noticing that on x86 "long double" didn't
get represented correctly: emitted sizes varied between directives
used and/or decimal vs hex input, and the emitted size was wrong
altogether for 64-bit (on ELF at least). But as happens frequently,
once you start touching code, you find more issues. As a result
this series then also adds support for BFloat16 and Half Precision
values on x86. By doing so to a fair part in common code, this
then allows removing the custom handling that was added to both
Arm bitnesses a while ago.

I suppose there may be other architectures which are similarly
affected, and where hence the new customization options may also
want making use of. This also extends to non-ELF x86, which I
leave as is not because I think handling is correct there, but
merely because I don't know where respective ABI aspects would be
spelled out.

Finally I got puzzled by insufficient distinction of NaN types
that can be specified as well as unnecessary (in my view) code
duplication between the handling of positive and negative
infinities.

01: x86: have non-PE/COFF BEOS be recognized as ELF
02: x86/ELF: fix .tfloat output
03: x86/ELF: fix .ds.x output
04: x86/ELF: fix .tfloat output with hex input
05: x86: introduce .hfloat directive
06: x86: introduce .bfloat16 directive
07: gas: make 2nd argument of .dcb.* consistently optional
08: Arm32: leave more .bfloat16 processing to common code
09: Arm64: leave .bfloat16 processing to common code
10: gas: support NaN flavors
11: gas: fold IEEE encoding of -Inf with that of +Inf

Jan

Comments

Alan Modra via Binutils Aug. 6, 2021, 3:19 p.m. | #1
Alan, Nick,

On 23.07.2021 08:49, Jan Beulich via Binutils wrote:
> This started out with me noticing that on x86 "long double" didn't

> get represented correctly: emitted sizes varied between directives

> used and/or decimal vs hex input, and the emitted size was wrong

> altogether for 64-bit (on ELF at least). But as happens frequently,

> once you start touching code, you find more issues. As a result

> this series then also adds support for BFloat16 and Half Precision

> values on x86. By doing so to a fair part in common code, this

> then allows removing the custom handling that was added to both

> Arm bitnesses a while ago.

> 

> I suppose there may be other architectures which are similarly

> affected, and where hence the new customization options may also

> want making use of. This also extends to non-ELF x86, which I

> leave as is not because I think handling is correct there, but

> merely because I don't know where respective ABI aspects would be

> spelled out.

> 

> Finally I got puzzled by insufficient distinction of NaN types

> that can be specified as well as unnecessary (in my view) code

> duplication between the handling of positive and negative

> infinities.

> 

> 01: x86: have non-PE/COFF BEOS be recognized as ELF

> 02: x86/ELF: fix .tfloat output

> 03: x86/ELF: fix .ds.x output

> 04: x86/ELF: fix .tfloat output with hex input

> 05: x86: introduce .hfloat directive

> 06: x86: introduce .bfloat16 directive

> 07: gas: make 2nd argument of .dcb.* consistently optional

> 08: Arm32: leave more .bfloat16 processing to common code

> 09: Arm64: leave .bfloat16 processing to common code

> 10: gas: support NaN flavors

> 11: gas: fold IEEE encoding of -Inf with that of +Inf


while I did get x86 side approval from H.J., only patch 1 could
reasonably go in without the additional approval by a general
maintainer (or Arm ones for the two Arm-specific patches). May I
ask for such, or comments towards changes I need to make to the
series?

Thanks, Jan
Alan Modra via Binutils Aug. 10, 2021, 11:37 a.m. | #2
Hi Jan,

> while I did get x86 side approval from H.J., only patch 1 could

> reasonably go in without the additional approval by a general

> maintainer (or Arm ones for the two Arm-specific patches). May I

> ask for such, or comments towards changes I need to make to the

> series?


Sorry - I am just back from PTO.  Anyway, if you have not already
received approval for this patch series then please consider the
whole lot approved.  I do not think that we need to bring in the
ARM maintainers specifically.  As far as I am concerned I know
that you do good work and will have tested the patches thoroughly.

Cheers
   Nick
Alan Modra via Binutils Aug. 11, 2021, 6:47 a.m. | #3
Nick,

On 10.08.2021 13:37, Nick Clifton wrote:
>> while I did get x86 side approval from H.J., only patch 1 could

>> reasonably go in without the additional approval by a general

>> maintainer (or Arm ones for the two Arm-specific patches). May I

>> ask for such, or comments towards changes I need to make to the

>> series?

> 

> Sorry - I am just back from PTO.  Anyway, if you have not already

> received approval for this patch series then please consider the

> whole lot approved.  I do not think that we need to bring in the

> ARM maintainers specifically.  As far as I am concerned I know

> that you do good work and will have tested the patches thoroughly.


thanks for both the trust and the approval. Irrespective of the
testing done, I'm not going to exclude that I've missed a target
where I might have broken an existing test. Of course as long as
I can repro any breakage, I'm certainly promising to try to take
care of any such further issues with the series.

The first few patches had in part extensive post-commit-message
remarks, pointing at possible further issues to address. Do you
perhaps have a view on any of these?

Thanks, Jan
Alan Modra via Binutils Aug. 12, 2021, 7:29 a.m. | #4
On Wed, Aug 11, 2021 at 08:47:20AM +0200, Jan Beulich wrote:
> I can repro any breakage, I'm certainly promising to try to take

> care of any such further issues with the series.


I'll take care of this one.  Fixes tic4x-coff FAIL: simple FP constants

	* testsuite/gas/all/float.s: Make NaN tests conditional on hasnan.
	* testsuite/gas/all/gas.exp: Define hasnan.

diff --git a/gas/testsuite/gas/all/float.s b/gas/testsuite/gas/all/float.s
index f401ed31a84..a84222cb0fb 100644
--- a/gas/testsuite/gas/all/float.s
+++ b/gas/testsuite/gas/all/float.s
@@ -8,9 +8,11 @@ foo:	.single	0r1.2345e+06
 	.dc.s 1
 	.dc.s 0f:1234
 	.dc.s Inf
+ .ifdef hasnan
 	.dc.s NaN
 	.dc.s QNaN
 	.dc.s SNaN
+ .endif
 	.dcb.s 1
 	.dcb.s 1, 1
 	.dcb.s 1, 0s:4321
@@ -19,9 +21,11 @@ foo:	.single	0r1.2345e+06
 	.dc.d 1
 	.dc.d 0d:1234
 	.dc.d +Inf
+ .ifdef hasnan
 	.dc.d -NaN
 	.dc.d +QNaN
 	.dc.d -SNaN
+ .endif
 	.dcb.d 1
 	.dcb.d 1, 1
 	.dcb.d 1, 0r:4321
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 389634f6165..e490d1f20ab 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -45,7 +45,12 @@ if { [istarget hppa*-*-*] || [istarget *c54x*-*-*] || [istarget mep*-*-*]} then
 # No floating point support in assembly code for CRIS and Z80.
 if { ![istarget cris-*-*] && ![istarget crisv32-*-*] 
      && ![istarget z80-*-*] } then {
-    gas_test "float.s" ""   "" "simple FP constants"
+    if { [istarget tic4x-*-*] } then {
+	set as_opt ""
+    } else {
+	set as_opt "--defsym hasnan=1"
+    }
+    gas_test "float.s" $as_opt "" "simple FP constants"
 }
 
 # This test is meaningless for the PA; the difference of two undefined


-- 
Alan Modra
Australia Development Lab, IBM
Alan Modra via Binutils Aug. 12, 2021, 10:36 a.m. | #5
On 12.08.2021 09:29, Alan Modra wrote:
> On Wed, Aug 11, 2021 at 08:47:20AM +0200, Jan Beulich wrote:

>> I can repro any breakage, I'm certainly promising to try to take

>> care of any such further issues with the series.

> 

> I'll take care of this one.  Fixes tic4x-coff FAIL: simple FP constants

> 

> 	* testsuite/gas/all/float.s: Make NaN tests conditional on hasnan.

> 	* testsuite/gas/all/gas.exp: Define hasnan.


Thanks. While tic4x-coff is among the targets in my test set, I'll
need to fix my scripting to call out build issues more prominently.
The reason I didn't spot the problem is that my code change didn't
build in the first place, which Nick already took care of (thanks
Nick). Hence no tests were run at all, and with that none could
have failed. I'm sorry.

Jan