PPC libgcc IEEE128 soft-fp exception/rounding fixes
Hi all,
I am fairly new to IBM and recently appointed maintainer of libdfp,
and work on glibc on ppc. This is my first foray into libgcc.
libdfp implements many common transcendental functions, overrides
type conversions between decimal float and other GCC types with more
optimized variants for dpd encoding.
While investigating some rounding issues with conversions, I tried out
the recent IEEE128 soft-fp support for PPC using a supporting compiler,
while building some of the soft-fp parts locally.
I ran into the following issues which I have attempted to correct with
the attached patch for rs6000/sfp-machine.h:
- FP_INIT_ROUNDMODE writes junk to the fpscr. I assume this should be
reading the fpscr and initializing the local rounding mode variable
declared via _FP_DECL_EX.
- FP_TRAPPING_EXCEPTIONS evaluates to zero where used. It seems like it
should return a bit field of FP_EX_* bits indicating which trap is
enabled. Likewise, when these bits are set in the fpscr, the trap is
enabled.
libgcc
* config/rs6000/sfp-machine.h:
(_FP_DECL_EX): Declare _fpsr as union of u64 and double.
(FP_TRAPPING_EXCEPTIONS): Remove this, FP_HANDLE_EXCEPTIONS
will do them implicitly later on.
(FP_INIT_ROUNDMODE): Read the fpscr instead of writing
mystery value.
(FP_ROUNDMODE): Update type of _fpscr.
diff --git a/libgcc/config/rs6000/sfp-machine.h b/libgcc/config/rs6000/sfp-machine.h
index 75d5e1a..4bc0040 100644
--- a/libgcc/config/rs6000/sfp-machine.h
+++ b/libgcc/config/rs6000/sfp-machine.h
@@ -130,9 +130,9 @@ void __sfp_handle_exceptions (int);
if (__builtin_expect (_fex, 0)) \
__sfp_handle_exceptions (_fex); \
} while (0);
-/* A set bit indicates an exception is masked and a clear bit indicates it is
- trapping. */
-# define FP_TRAPPING_EXCEPTIONS (~_fpscr & (FP_EX_ALL >> 22))
+
+/* A set bit indicates an exception is trapping. */
+# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
# define FP_RND_NEAREST 0x0
# define FP_RND_ZERO 0x1
@@ -141,16 +141,16 @@ void __sfp_handle_exceptions (int);
# define FP_RND_MASK 0x3
# define _FP_DECL_EX \
- unsigned long long _fpscr __attribute__ ((unused)) = FP_RND_NEAREST
+ union { unsigned long long i; double d; } _fpscr __attribute__ ((unused)) = \
+ { .i = FP_RND_NEAREST }
#define FP_INIT_ROUNDMODE \
do { \
-__asm__ __volatile__ ("mtfsf 255, %0" \
- : \
- : "f" (_fpscr)); \
+__asm__ __volatile__ ("mffs %0" \
+ : "=f" (_fpscr.d)); \
} while (0)
-# define FP_ROUNDMODE (_fpscr & FP_RND_MASK)
+# define FP_ROUNDMODE (_fpscr.i & FP_RND_MASK)
#endif /* !__FLOAT128__ */
/* Define ALIASNAME as a strong alias for NAME. */
Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes
On 02/17/2016 08:37 PM, Alan Modra wrote:
>> +/* A set bit indicates an exception is trapping. */
>> +# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
>
> why then a shift here, since FP_EX_* are defined as the actual
> register bits? Oh, I see. FP_EX_* are the status bits, and you want
> the enable bits. ie. bit 56 rather than bit 34, bit 57 rather than
> bit 35 and so on (bits numbered from 0 as msb). A comment to that
> effect might reduce head scratching.
Thanks for taking a look Alan, and Joseph. I've expanded the comment
to clarify the otherwise mysterious shift with the updated patch.
libgcc
* config/rs6000/sfp-machine.h:
(_FP_DECL_EX): Declare _fpsr as a union of u64 and double.
(FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping
exceptions.
(FP_INIT_ROUNDMODE): Read the fpscr instead of writing
a mystery value.
(FP_ROUNDMODE): Update the usage of _fpscr.
diff --git a/libgcc/config/rs6000/sfp-machine.h b/libgcc/config/rs6000/sfp-machine.h
index 75d5e1a..8dd8ea3 100644
--- a/libgcc/config/rs6000/sfp-machine.h
+++ b/libgcc/config/rs6000/sfp-machine.h
@@ -130,9 +130,13 @@ void __sfp_handle_exceptions (int);
if (__builtin_expect (_fex, 0)) \
__sfp_handle_exceptions (_fex); \
} while (0);
-/* A set bit indicates an exception is masked and a clear bit indicates it is
- trapping. */
-# define FP_TRAPPING_EXCEPTIONS (~_fpscr & (FP_EX_ALL >> 22))
+
+/* The FP_EX_* bits track whether the exception has occurred. This macro
+ must set the FP_EX_* bits of those exceptions which are configured to
+ trap. The FPSCR bit which indicates this is 22 ISA bits above the
+ respective FP_EX_* bit. Note, the ISA labels bits from msb to lsb,
+ so 22 ISA bits above is 22 bits below when counted from the lsb. */
+# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
# define FP_RND_NEAREST 0x0
# define FP_RND_ZERO 0x1
@@ -141,16 +145,16 @@ void __sfp_handle_exceptions (int);
# define FP_RND_MASK 0x3
# define _FP_DECL_EX \
- unsigned long long _fpscr __attribute__ ((unused)) = FP_RND_NEAREST
+ union { unsigned long long i; double d; } _fpscr __attribute__ ((unused)) = \
+ { .i = FP_RND_NEAREST }
#define FP_INIT_ROUNDMODE \
do { \
-__asm__ __volatile__ ("mtfsf 255, %0" \
- : \
- : "f" (_fpscr)); \
+__asm__ __volatile__ ("mffs %0" \
+ : "=f" (_fpscr.d)); \
} while (0)
-# define FP_ROUNDMODE (_fpscr & FP_RND_MASK)
+# define FP_ROUNDMODE (_fpscr.i & FP_RND_MASK)
#endif /* !__FLOAT128__ */
/* Define ALIASNAME as a strong alias for NAME. */
Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes
Hi David, Bill merged this into trunk last week. Is it okay to backport this to GCC 5? Thanks, Paul On 02/22/2016 05:51 PM, David Edelsohn wrote: > libgcc > * config/rs6000/sfp-machine.h: > (_FP_DECL_EX): Declare _fpsr as a union of u64 and double. > (FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping > exceptions. > (FP_INIT_ROUNDMODE): Read the fpscr instead of writing > a mystery value. > (FP_ROUNDMODE): Update the usage of _fpscr. > > Okay. > > Thanks, David >
[PATCH 2/2] rust: update usage of TARGET_AIX to TARGET_AIX_OS
This was noticed when fixing the gccgo usage of the macro, the
rust usage is very similar.
TARGET_AIX is defined as a non-zero value on linux/powerpc64le
which may cause unexpected behavior. TARGET_AIX_OS should be
used to toggle AIX specific behavior.
gcc/rust/ChangeLog:
* rust-object-export.cc [TARGET_AIX]: Rename and update
usage to TARGET_AIX_OS.
---
gcc/rust/rust-object-export.cc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/rust/rust-object-export.cc b/gcc/rust/rust-object-export.cc
index 1143c767784..f9a395f6964 100644
--- a/gcc/rust/rust-object-export.cc
+++ b/gcc/rust/rust-object-export.cc
@@ -46,8 +46,8 @@
#define RUST_EXPORT_SECTION_NAME ".rust_export"
#endif
-#ifndef TARGET_AIX
-#define TARGET_AIX 0
+#ifndef TARGET_AIX_OS
+#define TARGET_AIX_OS 0
#endif
/* Return whether or not GCC has reported any errors. */
@@ -91,7 +91,7 @@ rust_write_export_data (const char *bytes, unsigned int size)
{
gcc_assert (targetm_common.have_named_sections);
sec = get_section (RUST_EXPORT_SECTION_NAME,
-TARGET_AIX ? SECTION_EXCLUDE : SECTION_DEBUG, NULL);
+TARGET_AIX_OS ? SECTION_EXCLUDE : SECTION_DEBUG, NULL);
}
switch_to_section (sec);
--
2.31.1
[PATCH 1/2] go: update usage of TARGET_AIX to TARGET_AIX_OS
TARGET_AIX is defined to a non-zero value on linux and maybe other
powerpc64le targets. This leads to unexpected behavior such as
dropping the .go_export section when linking a shared library
on linux/powerpc64le.
Instead, use TARGET_AIX_OS to toggle AIX specific behavior.
Fixes golang/go#60798.
gcc/go/ChangeLog:
* go-backend.cc [TARGET_AIX]: Rename and update usage to
TARGET_AIX_OS.
* go-lang.cc: Likewise.
---
gcc/go/go-backend.cc | 6 +++---
gcc/go/go-lang.cc| 8
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/gcc/go/go-backend.cc b/gcc/go/go-backend.cc
index c6a1a2b7c18..6e2c919e829 100644
--- a/gcc/go/go-backend.cc
+++ b/gcc/go/go-backend.cc
@@ -45,8 +45,8 @@ along with GCC; see the file COPYING3. If not see
#define GO_EXPORT_SECTION_NAME ".go_export"
#endif
-#ifndef TARGET_AIX
-#define TARGET_AIX 0
+#ifndef TARGET_AIX_OS
+#define TARGET_AIX_OS 0
#endif
/* This file holds all the cases where the Go frontend needs
@@ -107,7 +107,7 @@ go_write_export_data (const char *bytes, unsigned int size)
{
gcc_assert (targetm_common.have_named_sections);
sec = get_section (GO_EXPORT_SECTION_NAME,
-TARGET_AIX ? SECTION_EXCLUDE : SECTION_DEBUG,
+TARGET_AIX_OS ? SECTION_EXCLUDE : SECTION_DEBUG,
NULL);
}
diff --git a/gcc/go/go-lang.cc b/gcc/go/go-lang.cc
index b6e8c37bf22..c6c147b20a5 100644
--- a/gcc/go/go-lang.cc
+++ b/gcc/go/go-lang.cc
@@ -39,8 +39,8 @@ along with GCC; see the file COPYING3. If not see
#include "go-c.h"
#include "go-gcc.h"
-#ifndef TARGET_AIX
-#define TARGET_AIX 0
+#ifndef TARGET_AIX_OS
+#define TARGET_AIX_OS 0
#endif
/* Language-dependent contents of a type. */
@@ -116,9 +116,9 @@ go_langhook_init (void)
args.compiling_runtime = go_compiling_runtime;
args.debug_escape_level = go_debug_escape_level;
args.debug_escape_hash = go_debug_escape_hash;
- args.nil_check_size_threshold = TARGET_AIX ? -1 : 4096;
+ args.nil_check_size_threshold = TARGET_AIX_OS ? -1 : 4096;
args.debug_optimization = go_debug_optimization;
- args.need_eqtype = TARGET_AIX ? true : false;
+ args.need_eqtype = TARGET_AIX_OS ? true : false;
args.linemap = go_get_linemap();
args.backend = go_get_backend();
go_create_gogo (&args);
--
2.31.1
Re: [PATCH 2/2] rust: update usage of TARGET_AIX to TARGET_AIX_OS
On 6/19/23 3:39 AM, Thomas Schwinge wrote: Hi Paul! On 2023-06-16T11:00:02-0500, "Paul E. Murphy via Gcc-patches" wrote: This was noticed when fixing the gccgo usage of the macro, the rust usage is very similar. TARGET_AIX is defined as a non-zero value on linux/powerpc64le which may cause unexpected behavior. TARGET_AIX_OS should be used to toggle AIX specific behavior. gcc/rust/ChangeLog: * rust-object-export.cc [TARGET_AIX]: Rename and update usage to TARGET_AIX_OS. I don't have rights to formally approve this GCC/Rust change, but I'll note that it follows "as obvious" (see <https://gcc.gnu.org/gitwrite.html#policies>, "Obvious fixes") to the corresponding GCC/Go change, which has been approved: <https://inbox.sourceware.org/cakoqz8wdwc7g5_jbnk1jvgchhiurceeamzb5bqrx_vzjejp...@mail.gmail.com>, and which is where this GCC/Rust code has been copied from, so I suggest you push both patches at once. Grüße Thomas Hi Thomas, Thank you for reviewing. I do not have commit access, so I cannot push this myself. If this is OK, could one of the rust maintainers push this patch? Thanks, Paul
