[3/4] Add ILP32 support to aarch64
Commit Message
On Fri, 2017-08-04 at 00:12 +0000, Joseph Myers wrote:
> On Thu, 3 Aug 2017, Wilco Dijkstra wrote:
>
> > The generic implementation may well be faster... I'm not sure where the
> > requirement of not raising inexact comes from (I don't see it in the definition
> > of lrint, and we generally don't care since inexact is set by almost every FP
> > calculation), but if it is absolutely required you'd special case values larger
> > than LONG_MAX.
> The requirement comes from lrint being bound to IEEE 754 conversion
> operations, so only raising inexact under the conditions specified and no
> spurious inexact.
Here is a new version of this patch. It (mostly) avoids fenv calls
when not needed and preserves any exceptions that may be set on entry
to the function.
Steve Ellcey
sellcey@cavium.com
2017-08-04 Steve Ellcey <sellcey@cavium.com>
* sysdeps/aarch64/fpu/s_llrint.c (OREG_SIZE): New macro.
* sysdeps/aarch64/fpu/s_llround.c (OREG_SIZE): Likewise.
* sysdeps/aarch64/fpu/s_llrintf.c (OREGS, IREGS): Remove.
(IREG_SIZE, OREG_SIZE): New macros.
* sysdeps/aarch64/fpu/s_llroundf.c: (OREGS, IREGS): Remove.
(IREG_SIZE, OREG_SIZE): New macros.
* sysdeps/aarch64/fpu/s_lrintf.c (IREGS): Remove.
(IREG_SIZE): New macro.
* sysdeps/aarch64/fpu/s_lroundf.c (IREGS): Remove.
(IREG_SIZE): New macro.
* sysdeps/aarch64/fpu/s_lrint.c (math_private.h, fenv.h, stdint.h):
New includes.
(IREG_SIZE, OREG_SIZE): Initialize if not already set.
(OREGS, IREGS): Set based on IREG_SIZE and OREG_SIZE.
(__CONCATX): Handle exceptions correctly on large values that may
set FE_INVALID.
* sysdeps/aarch64/fpu/s_lround.c (IREG_SIZE, OREG_SIZE):
Initialize if not already set.
(OREGS, IREGS): Set based on IREG_SIZE and OREG_SIZE.
Comments
On 05/08/17 00:15, Steve Ellcey wrote:
> On Fri, 2017-08-04 at 00:12 +0000, Joseph Myers wrote:
>> > On Thu, 3 Aug 2017, Wilco Dijkstra wrote:
>> >
>>> > > The generic implementation may well be faster... I'm not sure where the
>>> > > requirement of not raising inexact comes from (I don't see it in the definition
>>> > > of lrint, and we generally don't care since inexact is set by almost every FP
>>> > > calculation), but if it is absolutely required you'd special case values larger
>>> > > than LONG_MAX.
>> > The requirement comes from lrint being bound to IEEE 754 conversion
>> > operations, so only raising inexact under the conditions specified and no
>> > spurious inexact.
>
> Here is a new version of this patch. It (mostly) avoids fenv calls
> when not needed and preserves any exceptions that may be set on entry
> to the function.
>
...
> +#if IREG_SIZE == 64 && OREG_SIZE == 32
> + if (__builtin_fabs (x) > INT32_MAX - 2)
i don't understand the -2 here.
> + {
> + /* Converting large values to a 32 bit in may cause the frintx/fcvtza
s/in/int/
> + sequence to set both FE_INVALID and FE_INEXACT. To avoid this
> + we save and restore the FE and only set one or the other. */
> +
> + fenv_t env;
> + bool invalid_p, inexact_p;
> +
> + libc_feholdexcept (&env);
> + asm ( "frintx" "\t%" IREGS "1, %" IREGS "2\n\t"
> + "fcvtzs" "\t%" OREGS "0, %" IREGS "1"
> + : "=r" (result), "=w" (temp) : "w" (x) );
> + invalid_p = libc_fetestexcept (FE_INVALID);
> + inexact_p = libc_fetestexcept (FE_INEXACT);
multiple flags can be tested/raised in a single call.
> + libc_fesetenv (&env);
> +
> + if (invalid_p)
> + feraiseexcept (FE_INVALID);
> + else if (inexact_p)
> + feraiseexcept (FE_INEXACT);
> +
i think correct trapping is not guaranteed by glibc,
only correct status flags when the function returns,
so spurious inexact is not a problem if it is already
raised, and then i expect better code gen for the
inexact clearing approach:
if (fabs (x) > INT32_MAX && fetestexcept (FE_INEXACT) == 0)
{
asm (...);
if (fetestexcept (FE_INVALID|FE_INEXACT) == (FE_INVALID|FE_INEXACT))
feclearexcept (FE_INEXACT);
}
else
asm (...);
On 08/08/17 16:01, Szabolcs Nagy wrote:
> On 05/08/17 00:15, Steve Ellcey wrote:
>> On Fri, 2017-08-04 at 00:12 +0000, Joseph Myers wrote:
>>>> On Thu, 3 Aug 2017, Wilco Dijkstra wrote:
>>>>
>>>>>> The generic implementation may well be faster... I'm not sure where the
>>>>>> requirement of not raising inexact comes from (I don't see it in the definition
>>>>>> of lrint, and we generally don't care since inexact is set by almost every FP
>>>>>> calculation), but if it is absolutely required you'd special case values larger
>>>>>> than LONG_MAX.
>>>> The requirement comes from lrint being bound to IEEE 754 conversion
>>>> operations, so only raising inexact under the conditions specified and no
>>>> spurious inexact.
>>
>> Here is a new version of this patch. It (mostly) avoids fenv calls
>> when not needed and preserves any exceptions that may be set on entry
>> to the function.
>>
> ...
>> +#if IREG_SIZE == 64 && OREG_SIZE == 32
>> + if (__builtin_fabs (x) > INT32_MAX - 2)
>
> i don't understand the -2 here.
>
>> + {
>> + /* Converting large values to a 32 bit in may cause the frintx/fcvtza
>
> s/in/int/
>
>> + sequence to set both FE_INVALID and FE_INEXACT. To avoid this
>> + we save and restore the FE and only set one or the other. */
>> +
>> + fenv_t env;
>> + bool invalid_p, inexact_p;
>> +
>> + libc_feholdexcept (&env);
>> + asm ( "frintx" "\t%" IREGS "1, %" IREGS "2\n\t"
>> + "fcvtzs" "\t%" OREGS "0, %" IREGS "1"
>> + : "=r" (result), "=w" (temp) : "w" (x) );
>> + invalid_p = libc_fetestexcept (FE_INVALID);
>> + inexact_p = libc_fetestexcept (FE_INEXACT);
>
> multiple flags can be tested/raised in a single call.
>
>> + libc_fesetenv (&env);
>> +
>> + if (invalid_p)
>> + feraiseexcept (FE_INVALID);
>> + else if (inexact_p)
>> + feraiseexcept (FE_INEXACT);
>> +
>
> i think correct trapping is not guaranteed by glibc,
> only correct status flags when the function returns,
> so spurious inexact is not a problem if it is already
> raised, and then i expect better code gen for the
> inexact clearing approach:
>
> if (fabs (x) > INT32_MAX && fetestexcept (FE_INEXACT) == 0)
> {
> asm (...);
> if (fetestexcept (FE_INVALID|FE_INEXACT) == (FE_INVALID|FE_INEXACT))
> feclearexcept (FE_INEXACT);
Wilco pointed out to me that this approach would be
more complicated because invalid may be already raised
so you need to check that too, clear it if it's set
and restore it at the end..
> }
> else
> asm (...);
>
>
On Tue, 8 Aug 2017, Szabolcs Nagy wrote:
> i think correct trapping is not guaranteed by glibc,
> only correct status flags when the function returns,
> so spurious inexact is not a problem if it is already
> raised, and then i expect better code gen for the
> inexact clearing approach:
Since we have APIs for enabling / disabling exception traps, I think it's
expected that any spurious exceptions raised internally will be raised
inside an feholdexcept context so the user doesn't see traps. (We do not
claim anything about the number of times a given exception is raised
within a function, beyond whether it's zero or nonzero, or about the order
in which different exceptions are raised by a function raising multiple
exceptions, or about which subexceptions are raised on architectures such
as powerpc that support subexceptions.)
@@ -18,4 +18,5 @@
#define FUNC llrint
#define OTYPE long long int
+#define OREG_SIZE 64
#include <s_lrint.c>
@@ -18,6 +18,7 @@
#define FUNC llrintf
#define ITYPE float
-#define IREGS "s"
+#define IREG_SIZE 32
#define OTYPE long long int
+#define OREG_SIZE 64
#include <s_lrint.c>
@@ -18,4 +18,5 @@
#define FUNC llround
#define OTYPE long long int
+#define OREG_SIZE 64
#include <s_lround.c>
@@ -18,6 +18,7 @@
#define FUNC llroundf
#define ITYPE float
-#define IREGS "s"
+#define IREG_SIZE 32
#define OTYPE long long int
+#define OREG_SIZE 64
#include <s_lround.c>
@@ -16,7 +16,10 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
+#include <math_private.h>
#include <math.h>
+#include <fenv.h>
+#include <stdint.h>
#ifndef FUNC
# define FUNC lrint
@@ -24,18 +27,37 @@
#ifndef ITYPE
# define ITYPE double
-# define IREGS "d"
+# define IREG_SIZE 64
#else
-# ifndef IREGS
-# error IREGS not defined
+# ifndef IREG_SIZE
+# error IREG_SIZE not defined
# endif
#endif
#ifndef OTYPE
# define OTYPE long int
+# ifdef __ILP32__
+# define OREG_SIZE 32
+# else
+# define OREG_SIZE 64
+# endif
+#else
+# ifndef OREG_SIZE
+# error OREG_SIZE not defined
+# endif
+#endif
+
+#if IREG_SIZE == 32
+# define IREGS "s"
+#else
+# define IREGS "d"
#endif
-#define OREGS "x"
+#if OREG_SIZE == 32
+# define OREGS "w"
+#else
+# define OREGS "x"
+#endif
#define __CONCATX(a,b) __CONCAT(a,b)
@@ -44,6 +66,33 @@ __CONCATX(__,FUNC) (ITYPE x)
{
OTYPE result;
ITYPE temp;
+
+#if IREG_SIZE == 64 && OREG_SIZE == 32
+ if (__builtin_fabs (x) > INT32_MAX - 2)
+ {
+ /* Converting large values to a 32 bit in may cause the frintx/fcvtza
+ sequence to set both FE_INVALID and FE_INEXACT. To avoid this
+ we save and restore the FE and only set one or the other. */
+
+ fenv_t env;
+ bool invalid_p, inexact_p;
+
+ libc_feholdexcept (&env);
+ asm ( "frintx" "\t%" IREGS "1, %" IREGS "2\n\t"
+ "fcvtzs" "\t%" OREGS "0, %" IREGS "1"
+ : "=r" (result), "=w" (temp) : "w" (x) );
+ invalid_p = libc_fetestexcept (FE_INVALID);
+ inexact_p = libc_fetestexcept (FE_INEXACT);
+ libc_fesetenv (&env);
+
+ if (invalid_p)
+ feraiseexcept (FE_INVALID);
+ else if (inexact_p)
+ feraiseexcept (FE_INEXACT);
+
+ return result;
+ }
+#endif
asm ( "frintx" "\t%" IREGS "1, %" IREGS "2\n\t"
"fcvtzs" "\t%" OREGS "0, %" IREGS "1"
: "=r" (result), "=w" (temp) : "w" (x) );
@@ -18,5 +18,5 @@
#define FUNC lrintf
#define ITYPE float
-#define IREGS "s"
+#define IREG_SIZE 32
#include <s_lrint.c>
@@ -24,18 +24,37 @@
#ifndef ITYPE
# define ITYPE double
-# define IREGS "d"
+# define IREG_SIZE 64
#else
-# ifndef IREGS
-# error IREGS not defined
+# ifndef IREG_SIZE
+# error IREG_SIZE not defined
# endif
#endif
#ifndef OTYPE
# define OTYPE long int
+# ifdef __ILP32__
+# define OREG_SIZE 32
+# else
+# define OREG_SIZE 64
+# endif
+#else
+# ifndef OREG_SIZE
+# error OREG_SIZE not defined
+# endif
+#endif
+
+#if IREG_SIZE == 32
+# define IREGS "s"
+#else
+# define IREGS "d"
#endif
-#define OREGS "x"
+#if OREG_SIZE == 32
+# define OREGS "w"
+#else
+# define OREGS "x"
+#endif
#define __CONCATX(a,b) __CONCAT(a,b)
@@ -18,5 +18,5 @@
#define FUNC lroundf
#define ITYPE float
-#define IREGS "s"
+#define IREG_SIZE 32
#include <s_lround.c>