[COMMITTED] tilegx: enable wordsize-64 support for ieee745 dbl-64.

Message ID 5499E115.9000502@ezchip.com
State Superseded
Headers

Commit Message

Chris Metcalf Dec. 23, 2014, 9:39 p.m. UTC
  On 12/23/2014 4:21 PM, H.J. Lu wrote:
> On Tue, Dec 23, 2014 at 1:11 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
>> On 12/23/2014 3:47 PM, Chris Metcalf wrote:
>>> On 12/23/2014 3:06 PM, Joseph Myers wrote:
>>>> I take it that the ABI on tilegx32 allows the lround alias to llround
>>>
>>> No, it doesn't - I missed this possibility.
>>
>> Actually, it is, of course, a little trickier than that, since
>> we have to prevent s_llround.c from setting up the alias for
>> lround() as well.  But I think it may make sense to use global
>> conditional logic to do this.  The current users of dbl-64/wordsize-64
>> are aarch64, alpha, sparc64, and x86_64.  So as far as I know, the
>> question is whether the forthcoming ILP32 version of aarch64 would
>> benefit from this hack (probably yes) and whether the existing x32
>> version of x86_64 would (not clear to me, but plausibly better to clean
>> up the high bits of the returned value).
> Please make sure that x32 version is unchanged where
> lround is an alias of llround.

How about this version?  We only need to #define the symbol for the rare
case of !_LP64, but still using dbl-64/wordsize-64.  Not yet tested, so just
hoping for guidance with the model.

By the way, I could simplify the #if/#else body duplication if it's OK to
use an alias as the "source" of another alias, e.g. alias __llround to
__lround, then alias __lround to lround.  Is that OK?
  

Comments

H.J. Lu Dec. 23, 2014, 10:39 p.m. UTC | #1
On Tue, Dec 23, 2014 at 1:39 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> On 12/23/2014 4:21 PM, H.J. Lu wrote:
>>
>> On Tue, Dec 23, 2014 at 1:11 PM, Chris Metcalf <cmetcalf@ezchip.com>
>> wrote:
>>>
>>> On 12/23/2014 3:47 PM, Chris Metcalf wrote:
>>>>
>>>> On 12/23/2014 3:06 PM, Joseph Myers wrote:
>>>>>
>>>>> I take it that the ABI on tilegx32 allows the lround alias to llround
>>>>
>>>>
>>>> No, it doesn't - I missed this possibility.
>>>
>>>
>>> Actually, it is, of course, a little trickier than that, since
>>> we have to prevent s_llround.c from setting up the alias for
>>> lround() as well.  But I think it may make sense to use global
>>> conditional logic to do this.  The current users of dbl-64/wordsize-64
>>> are aarch64, alpha, sparc64, and x86_64.  So as far as I know, the
>>> question is whether the forthcoming ILP32 version of aarch64 would
>>> benefit from this hack (probably yes) and whether the existing x32
>>> version of x86_64 would (not clear to me, but plausibly better to clean
>>> up the high bits of the returned value).
>>
>> Please make sure that x32 version is unchanged where
>> lround is an alias of llround.
>
>
> How about this version?  We only need to #define the symbol for the rare
> case of !_LP64, but still using dbl-64/wordsize-64.  Not yet tested, so just
> hoping for guidance with the model.
>
> By the way, I could simplify the #if/#else body duplication if it's OK to
> use an alias as the "source" of another alias, e.g. alias __llround to
> __lround, then alias __lround to lround.  Is that OK?
>
> diff --git a/sysdeps/tile/sysdep.h b/sysdeps/tile/sysdep.h
> index 32aca49ff104..d56566ddcbfd 100644
> --- a/sysdeps/tile/sysdep.h
> +++ b/sysdeps/tile/sysdep.h
> @@ -66,6 +66,10 @@
>  #define REGSIZE                4
>  #endif
>
> +/* On tilegx, 32-bit values must have their high 32 bits sign extended;
> +   random values are not allowed.  */
> +#define USE_32BIT_NORMALIZE 1
> +
>  /* Support a limited form of shared assembly between tilepro and tilegx.
>     The presumption is that LD/ST are used for manipulating registers.
>     Since opcode parsing is case-insensitive, we don't need to provide
> diff --git a/sysdeps/x86_64/x32/sysdep.h b/sysdeps/x86_64/x32/sysdep.h
> index 7461827c83a8..c3ebc50be8c4 100644
> --- a/sysdeps/x86_64/x32/sysdep.h
> +++ b/sysdeps/x86_64/x32/sysdep.h
> @@ -90,3 +90,7 @@
>  # define R15_LP        "r15d"
>
>  #endif /* __ASSEMBLER__ */
> +
> +/* On x32, it is not required to normalize a 64-bit value before using
> +   it as a 32-bit value.  */
> +#define USE_32BIT_NORMALIZE 0
> diff --git a/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
> b/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
> index da180fef55cf..0cbc2d31588c 100644
> --- a/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
> +++ b/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
> @@ -64,16 +64,31 @@ __llround (double x)
>
>  weak_alias (__llround, llround)
>  #ifdef NO_LONG_DOUBLE
> -strong_alias (__llround, __lroundl)
> -weak_alias (__llround, lroundl)
> +strong_alias (__llround, __llroundl)
> +weak_alias (__llround, llroundl)
>  #endif
>
> -/* long has the same width as long long on 64-bit machines.  */
>  #undef lround
>  #undef __lround
> +/* long has the same width as long long on LP64 machines, so use an alias.
> +   If building for ILP32 on a machine with 64-bit registers, however,
> +   use a cast if necessary.  */
> +#if !defined (_LP64) && USE_32BIT_NORMALIZE
> +long int
> +__lround (double x)
> +{
> +  return __llround (x);
> +}
> +weak_alias (__lround, lround)
> +# ifdef NO_LONG_DOUBLE
> +strong_alias (__lround, __lroundl)
> +weak_alias (__lround, lroundl)
> +# endif
> +#else
>  strong_alias (__llround, __lround)
>  weak_alias (__llround, lround)
> -#ifdef NO_LONG_DOUBLE
> -strong_alias (__llround, __llroundl)
> -weak_alias (__llround, llroundl)
> +# ifdef NO_LONG_DOUBLE
> +strong_alias (__llround, __lroundl)
> +weak_alias (__llround, lroundl)
> +# endif
>  #endif
>

You can provide a safe default and let x32 override it.
  
Chris Metcalf Dec. 24, 2014, 5:10 p.m. UTC | #2
On 12/23/2014 5:39 PM, H.J. Lu wrote:
> You can provide a safe default and let x32 override it. 

I think it's conventional now to require that platforms are explicit
about #defines, and use #if with -Wundef.  It seems to make good sense
in this case, since it's really not clear what the right default for
any given platform is.

And since there are currently just two platforms that use wordsize-64
and are !_LP64 (x32 and tilegx32), it feels pretty reasonable to be
explicit for both.  The AArch64 guys will need to set the right default
for their ILP32 mode; I'm not even sure what the right answer is for them.
  
Andrew Pinski Dec. 24, 2014, 11:45 p.m. UTC | #3
On Wed, Dec 24, 2014 at 9:10 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> On 12/23/2014 5:39 PM, H.J. Lu wrote:
>>
>> You can provide a safe default and let x32 override it.
>
>
> I think it's conventional now to require that platforms are explicit
> about #defines, and use #if with -Wundef.  It seems to make good sense
> in this case, since it's really not clear what the right default for
> any given platform is.
>
> And since there are currently just two platforms that use wordsize-64
> and are !_LP64 (x32 and tilegx32), it feels pretty reasonable to be
> explicit for both.  The AArch64 guys will need to set the right default
> for their ILP32 mode; I'm not even sure what the right answer is for them.

AARCH64:ILP32 is fine with having the upper bits being undefined when
dealing with passing and return values.

Thanks,
Andrew

>
>
> --
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
>
  
Chris Metcalf Dec. 26, 2014, 3:06 p.m. UTC | #4
On 12/24/2014 6:45 PM, Andrew Pinski wrote:
> On Wed, Dec 24, 2014 at 9:10 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
>> On 12/23/2014 5:39 PM, H.J. Lu wrote:
>>> You can provide a safe default and let x32 override it.
>>
>> I think it's conventional now to require that platforms are explicit
>> about #defines, and use #if with -Wundef.  It seems to make good sense
>> in this case, since it's really not clear what the right default for
>> any given platform is.
>>
>> And since there are currently just two platforms that use wordsize-64
>> and are !_LP64 (x32 and tilegx32), it feels pretty reasonable to be
>> explicit for both.  The AArch64 guys will need to set the right default
>> for their ILP32 mode; I'm not even sure what the right answer is for them.
> AARCH64:ILP32 is fine with having the upper bits being undefined when
> dealing with passing and return values.

So sounds like two in each "camp", x32 and AArch64:ILP32 not wanting a cast,
and tilegx32 and MIPS needing the explicit cast, so requiring a
preprocessor symbol to be defined for that code seems like the right thing.

Any objections to the proposed code to do the extension in llround.c?
  
H.J. Lu Dec. 26, 2014, 3:44 p.m. UTC | #5
On Fri, Dec 26, 2014 at 7:06 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> On 12/24/2014 6:45 PM, Andrew Pinski wrote:
>>
>> On Wed, Dec 24, 2014 at 9:10 AM, Chris Metcalf <cmetcalf@ezchip.com>
>> wrote:
>>>
>>> On 12/23/2014 5:39 PM, H.J. Lu wrote:
>>>>
>>>> You can provide a safe default and let x32 override it.
>>>
>>>
>>> I think it's conventional now to require that platforms are explicit
>>> about #defines, and use #if with -Wundef.  It seems to make good sense
>>> in this case, since it's really not clear what the right default for
>>> any given platform is.
>>>
>>> And since there are currently just two platforms that use wordsize-64
>>> and are !_LP64 (x32 and tilegx32), it feels pretty reasonable to be
>>> explicit for both.  The AArch64 guys will need to set the right default
>>> for their ILP32 mode; I'm not even sure what the right answer is for
>>> them.
>>
>> AARCH64:ILP32 is fine with having the upper bits being undefined when
>> dealing with passing and return values.
>
>
> So sounds like two in each "camp", x32 and AArch64:ILP32 not wanting a cast,
> and tilegx32 and MIPS needing the explicit cast, so requiring a
> preprocessor symbol to be defined for that code seems like the right thing.
>
> Any objections to the proposed code to do the extension in llround.c?
>

Can we find a better name that USE_32BIT_NORMALIZE? How
about REGISTER_CAST_INT32_TO_INT64?  Also it should be an
error if the macro is undefined.
  
Chris Metcalf Dec. 26, 2014, 4:58 p.m. UTC | #6
On 12/26/2014 10:44 AM, H.J. Lu wrote:
> On Fri, Dec 26, 2014 at 7:06 AM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
>> So sounds like two in each "camp", x32 and AArch64:ILP32 not wanting a cast,
>> and tilegx32 and MIPS needing the explicit cast, so requiring a
>> preprocessor symbol to be defined for that code seems like the right thing.
>>
>> Any objections to the proposed code to do the extension in llround.c?
> Can we find a better name that USE_32BIT_NORMALIZE? How
> about REGISTER_CAST_INT32_TO_INT64?  Also it should be an
> error if the macro is undefined.

Sure, that name seems fine.  And since I'm proposing #if rather than
#ifdef, it will be an error with -Wundef -Werror if it is undefined.
  
Joseph Myers Dec. 31, 2014, 5:48 p.m. UTC | #7
On Tue, 23 Dec 2014, Chris Metcalf wrote:

> How about this version?  We only need to #define the symbol for the rare
> case of !_LP64, but still using dbl-64/wordsize-64.  Not yet tested, so just
> hoping for guidance with the model.

OK with HJ's proposed name for the macro (subject to testing).
  

Patch

diff --git a/sysdeps/tile/sysdep.h b/sysdeps/tile/sysdep.h
index 32aca49ff104..d56566ddcbfd 100644
--- a/sysdeps/tile/sysdep.h
+++ b/sysdeps/tile/sysdep.h
@@ -66,6 +66,10 @@ 
  #define REGSIZE                4
  #endif

+/* On tilegx, 32-bit values must have their high 32 bits sign extended;
+   random values are not allowed.  */
+#define USE_32BIT_NORMALIZE 1
+
  /* Support a limited form of shared assembly between tilepro and tilegx.
     The presumption is that LD/ST are used for manipulating registers.
     Since opcode parsing is case-insensitive, we don't need to provide
diff --git a/sysdeps/x86_64/x32/sysdep.h b/sysdeps/x86_64/x32/sysdep.h
index 7461827c83a8..c3ebc50be8c4 100644
--- a/sysdeps/x86_64/x32/sysdep.h
+++ b/sysdeps/x86_64/x32/sysdep.h
@@ -90,3 +90,7 @@ 
  # define R15_LP        "r15d"

  #endif /* __ASSEMBLER__ */
+
+/* On x32, it is not required to normalize a 64-bit value before using
+   it as a 32-bit value.  */
+#define USE_32BIT_NORMALIZE 0
diff --git a/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c b/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
index da180fef55cf..0cbc2d31588c 100644
--- a/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
+++ b/sysdeps/ieee754/dbl-64/wordsize-64/s_llround.c
@@ -64,16 +64,31 @@  __llround (double x)

  weak_alias (__llround, llround)
  #ifdef NO_LONG_DOUBLE
-strong_alias (__llround, __lroundl)
-weak_alias (__llround, lroundl)
+strong_alias (__llround, __llroundl)
+weak_alias (__llround, llroundl)
  #endif

-/* long has the same width as long long on 64-bit machines.  */
  #undef lround
  #undef __lround
+/* long has the same width as long long on LP64 machines, so use an alias.
+   If building for ILP32 on a machine with 64-bit registers, however,
+   use a cast if necessary.  */
+#if !defined (_LP64) && USE_32BIT_NORMALIZE
+long int
+__lround (double x)
+{
+  return __llround (x);
+}
+weak_alias (__lround, lround)
+# ifdef NO_LONG_DOUBLE
+strong_alias (__lround, __lroundl)
+weak_alias (__lround, lroundl)
+# endif
+#else
  strong_alias (__llround, __lround)
  weak_alias (__llround, lround)
-#ifdef NO_LONG_DOUBLE
-strong_alias (__llround, __llroundl)
-weak_alias (__llround, llroundl)
+# ifdef NO_LONG_DOUBLE
+strong_alias (__llround, __lroundl)
+weak_alias (__llround, lroundl)
+# endif
  #endif