Message ID | 1453901144-3662-1-git-send-email-gftg@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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
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 \ >
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 --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 \