diff mbox

powerpc: fenvinline.h refactor

Message ID 12b03cc3-0a23-ed07-e12d-8bdb6c7da9cb@linux.ibm.com
State Superseded
Delegated to: Adhemerval Zanella Netto
Headers

Commit Message

Rogerio Alves Jan. 23, 2020, 6:14 p.m. UTC 
  Hi,

Attached to this email is a patch to refactor fenvinline.h for power to 
include some __buitins when available. I also removed a weird asm 
constraint. I can't reproduce the warning described on the file and the 
test passed. Not sure why the put that constraint. I will appreciate any 
reviews.

Regard

Comments

Adhemerval Zanella Jan. 23, 2020, 9:24 p.m. UTC | #1 
On 23/01/2020 15:14, Rogerio Alves wrote:
> Hi,
> 
> Attached to this email is a patch to refactor fenvinline.h for power to include some __buitins when available. I also removed a weird asm constraint. I can't reproduce the warning described on the file and the test passed. Not sure why the put that constraint. I will appreciate any reviews.

PS: please send the patch inline next time, it helps the review
process.

> 
> Regard

> From dbeca754bcbc6c77b6157a2f818554db31bac2ba Mon Sep 17 00:00:00 2001
> From: Rogerio Alves <rcardoso@linux.ibm.com>
> Date: Thu, 23 Jan 2020 14:54:00 -0300
> Subject: [PATCH v1] powerpc: Refactor fenvinline.h
> 
> This patch refactor fenviline.h replaces some statements for
> builtins.
> 
> 2020-01-22  Rogerio A. Cardoso  <rcardoso@linux.ibm.com>
> 
> 	* sysdeps/unix/sysv/linux/powerpc/bits/fenvline.h:
>     Replace expression by __builtin_popcount. Use
>     __builtin_mtfsb[01] when avaliable. Remove weird i#*X
>     asm constraint.

No ChangeLog entry required anymore.

> ---
>  sysdeps/powerpc/bits/fenvinline.h | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
> index 70689664e2..ae04ece189 100644
> --- a/sysdeps/powerpc/bits/fenvinline.h
> +++ b/sysdeps/powerpc/bits/fenvinline.h
> @@ -15,7 +15,7 @@
>     You should have received a copy of the GNU Lesser General Public
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
> -
> +#include <stdio.h>

This is most likely not required.

>  #if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
>  
>  /* Inline definitions for fegetround.  */
> @@ -51,9 +51,16 @@
>  # define fegetround() __fegetround ()
>  
>  # ifndef __NO_MATH_INLINES
> -/* The weird 'i#*X' constraints on the following suppress a gcc
> -   warning when __excepts is not a constant.  Otherwise, they mean the
> -   same as just plain 'i'.  */

This is an installed header which can potentially be used with a older
gcc version. It is most likely an old bug fixed on newer release, so to
use the plain 'i' constrain you need to set it to use iff with a gcc
version where the warning surely does not happen. 

> +
> +/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
> +#  if !__GNUC_PREREQ(9, 0)
> +
> +#   define __builtin_mtfsb0(__b)					\
> +   __asm__ __volatile__ ("mtfsb0 %0" : : "i" __b)
> +
> +#   define __builtin_mtfsb1(__b)					\
> +   __asm__ __volatile__ ("mtfsb1 %0" : : "i" __b)
> +#  endif

I don't think it is a good practice to re-define bultins, use it along with 
an extra indirection instead.  Something as:

  #  if !__GNUC_PREREQ(9, 0)
  #   define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i" (__b))
  #   define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i" (__b))
  #  else
  #   define __mtfsb0(__b) __builtin_mtfsb0 (__b)
  #   define __mtfsb1(__b) __builtin_mtfsb1 (__b)
  #  endif

>  
>  #  if __GNUC_PREREQ(3, 4)
>  
> @@ -63,12 +70,11 @@
>      int __e = __excepts;						      \
>      int __ret;								      \
>      if (__builtin_constant_p (__e)					      \
> -        && (__e & (__e - 1)) == 0					      \
> +        && (__builtin_popcount (__e) == 1)				      \

Why change this code? The idea is not emit any of the check for constant
call with only one FE_* flag (either mtfsb0 or mtfsb1).

>          && __e != FE_INVALID)						      \
>        {									      \
>  	if (__e != 0)							      \
> -	  __asm__ __volatile__ ("mtfsb1 %0"				      \
> -				: : "i#*X" (__builtin_clz (__e)));	      \
> +	  __builtin_mtfsb1 ((__builtin_clz (__e)));			      \
>          __ret = 0;							      \
>        }									      \
>      else								      \
> @@ -82,12 +88,11 @@
>      int __e = __excepts;						      \
>      int __ret;								      \
>      if (__builtin_constant_p (__e)					      \
> -        && (__e & (__e - 1)) == 0					      \
> +        && (__builtin_popcount (__e) == 1)				      \
>          && __e != FE_INVALID)						      \
>        {									      \
>  	if (__e != 0)							      \
> -	  __asm__ __volatile__ ("mtfsb0 %0"				      \
> -				: : "i#*X" (__builtin_clz (__e)));	      \
> +	  __builtin_mtfsb0 ((__builtin_clz (__e)));			      \
>          __ret = 0;							      \
>        }									      \
>      else								      \
> -- 
> 2.17.1
Florian Weimer Jan. 24, 2020, 9:50 a.m. UTC | #2 
* Adhemerval Zanella:

> I don't think it is a good practice to re-define bultins, use it along with 
> an extra indirection instead.  Something as:
>
>   #  if !__GNUC_PREREQ(9, 0)
>   #   define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i" (__b))
>   #   define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i" (__b))
>   #  else
>   #   define __mtfsb0(__b) __builtin_mtfsb0 (__b)
>   #   define __mtfsb1(__b) __builtin_mtfsb1 (__b)
>   #  endif

I think __mtfsb0 and __mtfsb1 conflict with gcc's installed
pu_intrinsics.h header.

Thanks,
Florian
Adhemerval Zanella Jan. 24, 2020, 12:16 p.m. UTC | #3 
On 24/01/2020 06:50, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> I don't think it is a good practice to re-define bultins, use it along with 
>> an extra indirection instead.  Something as:
>>
>>   #  if !__GNUC_PREREQ(9, 0)
>>   #   define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i" (__b))
>>   #   define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i" (__b))
>>   #  else
>>   #   define __mtfsb0(__b) __builtin_mtfsb0 (__b)
>>   #   define __mtfsb1(__b) __builtin_mtfsb1 (__b)
>>   #  endif
> 
> I think __mtfsb0 and __mtfsb1 conflict with gcc's installed
> pu_intrinsics.h header.

Right, in this case prepend with __glibc or any other related unique
identifier.  Checking on ppu_intrinsics.h, I wonder if the asm inline
should not use the 'n' constraint as well.
Rogerio Alves Jan. 27, 2020, 2:05 p.m. UTC | #4 
> Subject: Re: [PATCH] powerpc: fenvinline.h refactor > Date: Thu, 23 Jan 2020 18:24:38 -0300
> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> To: libc-alpha@sourceware.org
> 
> 
> 
> On 23/01/2020 15:14, Rogerio Alves wrote:
>> Hi,
>>
>> Attached to this email is a patch to refactor fenvinline.h for power to include some __buitins when available. I also removed a weird asm constraint. I can't reproduce the warning described on the file and the test passed. Not sure why the put that constraint. I will appreciate any reviews.
> 
> PS: please send the patch inline next time, it helps the review
> process.
>

Ok. Once I include you suggestions I will send a inline patch instead.

>>
>> Regard
> 
>>  From dbeca754bcbc6c77b6157a2f818554db31bac2ba Mon Sep 17 00:00:00 2001
>> From: Rogerio Alves <rcardoso@linux.ibm.com>
>> Date: Thu, 23 Jan 2020 14:54:00 -0300
>> Subject: [PATCH v1] powerpc: Refactor fenvinline.h
>>
>> This patch refactor fenviline.h replaces some statements for
>> builtins.
>>
>> 2020-01-22  Rogerio A. Cardoso  <rcardoso@linux.ibm.com>
>>
>> 	* sysdeps/unix/sysv/linux/powerpc/bits/fenvline.h:
>>      Replace expression by __builtin_popcount. Use
>>      __builtin_mtfsb[01] when avaliable. Remove weird i#*X
>>      asm constraint.
> 
> No ChangeLog entry required anymore.
> 

Oh. I was not aware. Last time I sent a patch here was still required. I 
will remove.

>> ---
>>   sysdeps/powerpc/bits/fenvinline.h | 25 +++++++++++++++----------
>>   1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
>> index 70689664e2..ae04ece189 100644
>> --- a/sysdeps/powerpc/bits/fenvinline.h
>> +++ b/sysdeps/powerpc/bits/fenvinline.h
>> @@ -15,7 +15,7 @@
>>      You should have received a copy of the GNU Lesser General Public
>>      License along with the GNU C Library; if not, see
>>      <https://www.gnu.org/licenses/>.  */
>> -
>> +#include <stdio.h>
> 
> This is most likely not required.
> 

Yes. I forgot to remove that.

>>   #if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
>>   
>>   /* Inline definitions for fegetround.  */
>> @@ -51,9 +51,16 @@
>>   # define fegetround() __fegetround ()
>>   
>>   # ifndef __NO_MATH_INLINES
>> -/* The weird 'i#*X' constraints on the following suppress a gcc
>> -   warning when __excepts is not a constant.  Otherwise, they mean the
>> -   same as just plain 'i'.  */
> 
> This is an installed header which can potentially be used with a older
> gcc version. It is most likely an old bug fixed on newer release, so to
> use the plain 'i' constrain you need to set it to use iff with a gcc
> version where the warning surely does not happen.
>

Yes. I will check the minimal gcc version where the warning do not 
happen and set it. I am working on that here.

>> +
>> +/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
>> +#  if !__GNUC_PREREQ(9, 0)
>> +
>> +#   define __builtin_mtfsb0(__b)					\
>> +   __asm__ __volatile__ ("mtfsb0 %0" : : "i" __b)
>> +
>> +#   define __builtin_mtfsb1(__b)					\
>> +   __asm__ __volatile__ ("mtfsb1 %0" : : "i" __b)
>> +#  endif
> 
> I don't think it is a good practice to re-define bultins, use it along with
> an extra indirection instead.  Something as:
>
>    #  if !__GNUC_PREREQ(9, 0)
>    #   define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i" (__b))
>    #   define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i" (__b))
>    #  else
>    #   define __mtfsb0(__b) __builtin_mtfsb0 (__b)
>    #   define __mtfsb1(__b) __builtin_mtfsb1 (__b)
>    #  endif
>

Nice. I'll do that. Thank you for this suggestion.

>>   
>>   #  if __GNUC_PREREQ(3, 4)
>>   
>> @@ -63,12 +70,11 @@
>>       int __e = __excepts;						      \
>>       int __ret;								      \
>>       if (__builtin_constant_p (__e)					      \
>> -        && (__e & (__e - 1)) == 0					      \
>> +        && (__builtin_popcount (__e) == 1)				      \
> 
> Why change this code? The idea is not emit any of the check for constant
> call with only one FE_* flag (either mtfsb0 or mtfsb1).
> 

In that case the test (__e & (__e - 1)) == 0 tests if only one bit is 
set on __e. Since FE_* flags are in power of two (except FE_ALL_EXCEPT) 
it has the same effect that __builtin_popcount. I guess is more readable 
to use __builtin_popcount(__e) == 1. The code is still the same: not 
emit any of the check for constant call it only one FE_ flag is set.

>>           && __e != FE_INVALID)						      \
>>         {									      \
>>   	if (__e != 0)							      \
>> -	  __asm__ __volatile__ ("mtfsb1 %0"				      \
>> -				: : "i#*X" (__builtin_clz (__e)));	      \
>> +	  __builtin_mtfsb1 ((__builtin_clz (__e)));			      \
>>           __ret = 0;							      \
>>         }									      \
>>       else								      \
>> @@ -82,12 +88,11 @@
>>       int __e = __excepts;						      \
>>       int __ret;								      \
>>       if (__builtin_constant_p (__e)					      \
>> -        && (__e & (__e - 1)) == 0					      \
>> +        && (__builtin_popcount (__e) == 1)				      \
>>           && __e != FE_INVALID)						      \
>>         {									      \
>>   	if (__e != 0)							      \
>> -	  __asm__ __volatile__ ("mtfsb0 %0"				      \
>> -				: : "i#*X" (__builtin_clz (__e)));	      \
>> +	  __builtin_mtfsb0 ((__builtin_clz (__e)));			      \
>>           __ret = 0;							      \
>>         }									      \
>>       else								      \
>> -- 
>> 2.17.1
Adhemerval Zanella Jan. 27, 2020, 3:11 p.m. UTC | #5 
On 27/01/2020 11:05, Rogerio Alves wrote:
> 
>> Subject: Re: [PATCH] powerpc: fenvinline.h refactor > Date: Thu, 23 Jan 2020 18:24:38 -0300
>> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> To: libc-alpha@sourceware.org
>>
>>
>>
>> On 23/01/2020 15:14, Rogerio Alves wrote:
>>> Hi,
>>>
>>> Attached to this email is a patch to refactor fenvinline.h for power to include some __buitins when available. I also removed a weird asm constraint. I can't reproduce the warning described on the file and the test passed. Not sure why the put that constraint. I will appreciate any reviews.
>>
>> PS: please send the patch inline next time, it helps the review
>> process.
>>
> 
> Ok. Once I include you suggestions I will send a inline patch instead.
> 
>>>
>>> Regard
>>
>>>  From dbeca754bcbc6c77b6157a2f818554db31bac2ba Mon Sep 17 00:00:00 2001
>>> From: Rogerio Alves <rcardoso@linux.ibm.com>
>>> Date: Thu, 23 Jan 2020 14:54:00 -0300
>>> Subject: [PATCH v1] powerpc: Refactor fenvinline.h
>>>
>>> This patch refactor fenviline.h replaces some statements for
>>> builtins.
>>>
>>> 2020-01-22  Rogerio A. Cardoso  <rcardoso@linux.ibm.com>
>>>
>>>     * sysdeps/unix/sysv/linux/powerpc/bits/fenvline.h:
>>>      Replace expression by __builtin_popcount. Use
>>>      __builtin_mtfsb[01] when avaliable. Remove weird i#*X
>>>      asm constraint.
>>
>> No ChangeLog entry required anymore.
>>
> 
> Oh. I was not aware. Last time I sent a patch here was still required. I will remove.
> 
>>> ---
>>>   sysdeps/powerpc/bits/fenvinline.h | 25 +++++++++++++++----------
>>>   1 file changed, 15 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
>>> index 70689664e2..ae04ece189 100644
>>> --- a/sysdeps/powerpc/bits/fenvinline.h
>>> +++ b/sysdeps/powerpc/bits/fenvinline.h
>>> @@ -15,7 +15,7 @@
>>>      You should have received a copy of the GNU Lesser General Public
>>>      License along with the GNU C Library; if not, see
>>>      <https://www.gnu.org/licenses/>.  */
>>> -
>>> +#include <stdio.h>
>>
>> This is most likely not required.
>>
> 
> Yes. I forgot to remove that.
> 
>>>   #if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
>>>     /* Inline definitions for fegetround.  */
>>> @@ -51,9 +51,16 @@
>>>   # define fegetround() __fegetround ()
>>>     # ifndef __NO_MATH_INLINES
>>> -/* The weird 'i#*X' constraints on the following suppress a gcc
>>> -   warning when __excepts is not a constant.  Otherwise, they mean the
>>> -   same as just plain 'i'.  */
>>
>> This is an installed header which can potentially be used with a older
>> gcc version. It is most likely an old bug fixed on newer release, so to
>> use the plain 'i' constrain you need to set it to use iff with a gcc
>> version where the warning surely does not happen.
>>
> 
> Yes. I will check the minimal gcc version where the warning do not happen and set it. I am working on that here.
> 
>>> +
>>> +/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
>>> +#  if !__GNUC_PREREQ(9, 0)
>>> +
>>> +#   define __builtin_mtfsb0(__b)                    \
>>> +   __asm__ __volatile__ ("mtfsb0 %0" : : "i" __b)
>>> +
>>> +#   define __builtin_mtfsb1(__b)                    \
>>> +   __asm__ __volatile__ ("mtfsb1 %0" : : "i" __b)
>>> +#  endif
>>
>> I don't think it is a good practice to re-define bultins, use it along with
>> an extra indirection instead.  Something as:
>>
>>    #  if !__GNUC_PREREQ(9, 0)
>>    #   define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i" (__b))
>>    #   define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i" (__b))
>>    #  else
>>    #   define __mtfsb0(__b) __builtin_mtfsb0 (__b)
>>    #   define __mtfsb1(__b) __builtin_mtfsb1 (__b)
>>    #  endif
>>
> 
> Nice. I'll do that. Thank you for this suggestion.

Also, __mtfsb{0,1} definition clash with ppu_intrinsics.h (which is an
installed gcc header). So use __glibc_mtfsb{0,1} instead.

> 
>>>     #  if __GNUC_PREREQ(3, 4)
>>>   @@ -63,12 +70,11 @@
>>>       int __e = __excepts;                              \
>>>       int __ret;                                      \
>>>       if (__builtin_constant_p (__e)                          \
>>> -        && (__e & (__e - 1)) == 0                          \
>>> +        && (__builtin_popcount (__e) == 1)                      \
>>
>> Why change this code? The idea is not emit any of the check for constant
>> call with only one FE_* flag (either mtfsb0 or mtfsb1).
>>
> 
> In that case the test (__e & (__e - 1)) == 0 tests if only one bit is set on __e. Since FE_* flags are in power of two (except FE_ALL_EXCEPT) it has the same effect that __builtin_popcount. I guess is more readable to use __builtin_popcount(__e) == 1. The code is still the same: not emit any of the check for constant call it only one FE_ flag is set.

My understanding it builds on the assumption that all the FE_* flags
are multiple of 2, meaning only on bit will be set and thus if the
exception to be set is not an multiple of 2 then it can not be set/unset 
using mtfsb{0,1}.

This change is essentially the same, so I don't see a strong motivation 
to do it.

> 
>>>           && __e != FE_INVALID)                              \
>>>         {                                          \
>>>       if (__e != 0)                                  \
>>> -      __asm__ __volatile__ ("mtfsb1 %0"                      \
>>> -                : : "i#*X" (__builtin_clz (__e)));          \
>>> +      __builtin_mtfsb1 ((__builtin_clz (__e)));                  \
>>>           __ret = 0;                                  \
>>>         }                                          \
>>>       else                                      \
>>> @@ -82,12 +88,11 @@
>>>       int __e = __excepts;                              \
>>>       int __ret;                                      \
>>>       if (__builtin_constant_p (__e)                          \
>>> -        && (__e & (__e - 1)) == 0                          \
>>> +        && (__builtin_popcount (__e) == 1)                      \
>>>           && __e != FE_INVALID)                              \
>>>         {                                          \
>>>       if (__e != 0)                                  \
>>> -      __asm__ __volatile__ ("mtfsb0 %0"                      \
>>> -                : : "i#*X" (__builtin_clz (__e)));          \
>>> +      __builtin_mtfsb0 ((__builtin_clz (__e)));                  \
>>>           __ret = 0;                                  \
>>>         }                                          \
>>>       else                                      \
>>> -- 
>>> 2.17.1
diff mbox

Patch

From dbeca754bcbc6c77b6157a2f818554db31bac2ba Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.ibm.com>
Date: Thu, 23 Jan 2020 14:54:00 -0300
Subject: [PATCH v1] powerpc: Refactor fenvinline.h

This patch refactor fenviline.h replaces some statements for
builtins.

2020-01-22  Rogerio A. Cardoso  <rcardoso@linux.ibm.com>

	* sysdeps/unix/sysv/linux/powerpc/bits/fenvline.h:
    Replace expression by __builtin_popcount. Use
    __builtin_mtfsb[01] when avaliable. Remove weird i#*X
    asm constraint.
---
 sysdeps/powerpc/bits/fenvinline.h | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
index 70689664e2..ae04ece189 100644
--- a/sysdeps/powerpc/bits/fenvinline.h
+++ b/sysdeps/powerpc/bits/fenvinline.h
@@ -15,7 +15,7 @@ 
    You should have received a copy of the GNU Lesser General Public
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
-
+#include <stdio.h>
 #if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
 
 /* Inline definitions for fegetround.  */
@@ -51,9 +51,16 @@ 
 # define fegetround() __fegetround ()
 
 # ifndef __NO_MATH_INLINES
-/* The weird 'i#*X' constraints on the following suppress a gcc
-   warning when __excepts is not a constant.  Otherwise, they mean the
-   same as just plain 'i'.  */
+
+/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
+#  if !__GNUC_PREREQ(9, 0)
+
+#   define __builtin_mtfsb0(__b)					\
+   __asm__ __volatile__ ("mtfsb0 %0" : : "i" __b)
+
+#   define __builtin_mtfsb1(__b)					\
+   __asm__ __volatile__ ("mtfsb1 %0" : : "i" __b)
+#  endif
 
 #  if __GNUC_PREREQ(3, 4)
 
@@ -63,12 +70,11 @@ 
     int __e = __excepts;						      \
     int __ret;								      \
     if (__builtin_constant_p (__e)					      \
-        && (__e & (__e - 1)) == 0					      \
+        && (__builtin_popcount (__e) == 1)				      \
         && __e != FE_INVALID)						      \
       {									      \
 	if (__e != 0)							      \
-	  __asm__ __volatile__ ("mtfsb1 %0"				      \
-				: : "i#*X" (__builtin_clz (__e)));	      \
+	  __builtin_mtfsb1 ((__builtin_clz (__e)));			      \
         __ret = 0;							      \
       }									      \
     else								      \
@@ -82,12 +88,11 @@ 
     int __e = __excepts;						      \
     int __ret;								      \
     if (__builtin_constant_p (__e)					      \
-        && (__e & (__e - 1)) == 0					      \
+        && (__builtin_popcount (__e) == 1)				      \
         && __e != FE_INVALID)						      \
       {									      \
 	if (__e != 0)							      \
-	  __asm__ __volatile__ ("mtfsb0 %0"				      \
-				: : "i#*X" (__builtin_clz (__e)));	      \
+	  __builtin_mtfsb0 ((__builtin_clz (__e)));			      \
         __ret = 0;							      \
       }									      \
     else								      \
-- 
2.17.1