diff mbox

powerpc: Remove uses of operand modifier (%s) in inline asm

Message ID 1453901144-3662-1-git-send-email-gftg@linux.vnet.ibm.com
State New
Headers show

Commit Message

Gabriel F. T. Gomes Jan. 27, 2016, 1:25 p.m. UTC
The operand modifier %s on powerpc is an undocumented internal implementation
detail of GCC.  Besides that, the GCC community wants to remove it.  This patch
rewrites the expressions that use this modifier with logically equivalent
expressions that don't require it.

Explanation for the substitution:

The %s modifier takes an immediate operand and prints 32 less such immediate.
Thus, in the previous code, the expression resulted in:

  32 - __builtin_ffs(e)

where e was guaranteed to have exactly a single bit set, by the following
expressions:

  (e & (e-1) == 0) : e has at most one bit set.
  (e != 0)         : e is not zero, thus it has at least one bit set.

Since we guarantee that there is exactly only one bit set, the following
statement is true:

  32 - __builtin_ffs(e) == __builtin_clz(e)

Thus, we can replace __builtin_ffs with __builtin_clz and remove the %s operand
modifier.

2016-01-25  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>

	* sysdeps/powerpc/bits/fenvinline.h (feraiseexcept): Remove use of %s
	operand modifier.
	(feclearexcept): Likewise.
---
 sysdeps/powerpc/bits/fenvinline.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Gabriel F. T. Gomes Jan. 27, 2016, 1:30 p.m. UTC | #1
Oops.

I forgot to mention that this is for 2.24 and that I tested it on ppc,
ppc64, and ppc64le.

Sorry about that.

Kind regards,
Gabriel
Adhemerval Zanella Jan. 27, 2016, 1:51 p.m. UTC | #2
GCC developers seems to have included this modifier back [1], should we really
need it (is there a released version that do not support it)? 

[1] https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01937.html

On 27-01-2016 11:25, Gabriel F. T. Gomes wrote:
> The operand modifier %s on powerpc is an undocumented internal implementation
> detail of GCC.  Besides that, the GCC community wants to remove it.  This patch
> rewrites the expressions that use this modifier with logically equivalent
> expressions that don't require it.
> 
> Explanation for the substitution:
> 
> The %s modifier takes an immediate operand and prints 32 less such immediate.
> Thus, in the previous code, the expression resulted in:
> 
>   32 - __builtin_ffs(e)
> 
> where e was guaranteed to have exactly a single bit set, by the following
> expressions:
> 
>   (e & (e-1) == 0) : e has at most one bit set.
>   (e != 0)         : e is not zero, thus it has at least one bit set.
> 
> Since we guarantee that there is exactly only one bit set, the following
> statement is true:
> 
>   32 - __builtin_ffs(e) == __builtin_clz(e)
> 
> Thus, we can replace __builtin_ffs with __builtin_clz and remove the %s operand
> modifier.
> 
> 2016-01-25  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>
> 
> 	* sysdeps/powerpc/bits/fenvinline.h (feraiseexcept): Remove use of %s
> 	operand modifier.
> 	(feclearexcept): Likewise.
> ---
>  sysdeps/powerpc/bits/fenvinline.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
> index 4a7b2af..7d7938e 100644
> --- a/sysdeps/powerpc/bits/fenvinline.h
> +++ b/sysdeps/powerpc/bits/fenvinline.h
> @@ -42,8 +42,8 @@
>          && __e != FE_INVALID)						      \
>        {									      \
>  	if (__e != 0)							      \
> -	  __asm__ __volatile__ ("mtfsb1 %s0"				      \
> -				: : "i#*X" (__builtin_ffs (__e)));	      \
> +	  __asm__ __volatile__ ("mtfsb1 %0"				      \
> +				: : "i#*X" (__builtin_clz (__e)));	      \
>          __ret = 0;							      \
>        }									      \
>      else								      \
> @@ -61,8 +61,8 @@
>          && __e != FE_INVALID)						      \
>        {									      \
>  	if (__e != 0)							      \
> -	  __asm__ __volatile__ ("mtfsb0 %s0"				      \
> -				: : "i#*X" (__builtin_ffs (__e)));	      \
> +	  __asm__ __volatile__ ("mtfsb0 %0"				      \
> +				: : "i#*X" (__builtin_clz (__e)));	      \
>          __ret = 0;							      \
>        }									      \
>      else								      \
>
Joseph Myers Jan. 27, 2016, 5:19 p.m. UTC | #3
On Wed, 27 Jan 2016, Gabriel F. T. Gomes wrote:

> Thus, we can replace __builtin_ffs with __builtin_clz and remove the %s operand
> modifier.

Doesn't that mean you need to make these definitions conditional on 
__GNUC_PREREQ (3, 4) (which I think should be fine to do - we shouldn't 
need to care about these optimizations for pre-3.4 compilers), as that was 
the version where __builtin_clz was introduced?
diff mbox

Patch

diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
index 4a7b2af..7d7938e 100644
--- a/sysdeps/powerpc/bits/fenvinline.h
+++ b/sysdeps/powerpc/bits/fenvinline.h
@@ -42,8 +42,8 @@ 
         && __e != FE_INVALID)						      \
       {									      \
 	if (__e != 0)							      \
-	  __asm__ __volatile__ ("mtfsb1 %s0"				      \
-				: : "i#*X" (__builtin_ffs (__e)));	      \
+	  __asm__ __volatile__ ("mtfsb1 %0"				      \
+				: : "i#*X" (__builtin_clz (__e)));	      \
         __ret = 0;							      \
       }									      \
     else								      \
@@ -61,8 +61,8 @@ 
         && __e != FE_INVALID)						      \
       {									      \
 	if (__e != 0)							      \
-	  __asm__ __volatile__ ("mtfsb0 %s0"				      \
-				: : "i#*X" (__builtin_ffs (__e)));	      \
+	  __asm__ __volatile__ ("mtfsb0 %0"				      \
+				: : "i#*X" (__builtin_clz (__e)));	      \
         __ret = 0;							      \
       }									      \
     else								      \