Message ID | 20200207143410.3948-1-rcardoso@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [v2] powerpc: Refactor fenvinline.h | expand |
On 2/7/20 8:34 AM, Rogerio Alves wrote: > This patch refactor fenviline.h replaces some statements for > builtins. > > --- > > * Changes in v2: Per Adhemerval review: don not redefine _buitins use a > macro instead. Use of __builtin_popcount allows do eliminate the > if (e == 0) test. __builtin_popcount(e) == 1 for e = 0 validate false > while (e & (e - 1)) == 0 validate true if e = 0. Appended a comment > about the weird asm constrain i#*X. The warning only appear at GCC > versions 3 and below. If we even stops to support GCC 3 for installed > headers we may replace that for plain `i`. > sysdeps/powerpc/bits/fenvinline.h | 30 +++++++++++++++++------------- > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h > index 70689664e2..7b2b75e5ef 100644 > --- a/sysdeps/powerpc/bits/fenvinline.h > +++ b/sysdeps/powerpc/bits/fenvinline.h > @@ -51,9 +51,19 @@ > # define fegetround() __fegetround () > > # ifndef __NO_MATH_INLINES > + > +/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9. */ > +# if !__GNUC_PREREQ(9, 0) > /* 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'. */ > + same as just plain 'i'. This warning only happens in old GCC > + versions (gcc 3 or less). Otherwise plain 'i' works fine. */ > +# define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i#*X" (__b)) > +# define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i#*X" (__b)) > +# else > +# define __mtfsb0(__b) __builtin_mtfsb0 (__b) > +# define __mtfsb1(__b) __builtin_mtfsb1 (__b) > +# endif These names actually conflict with XL compiler builtins which serve the same purpose. https://www.ibm.com/support/knowledgecenter/SSXVZZ_16.1.1/com.ibm.xlcpp1611.lelinux.doc/compiler_ref/bifs_fpscr.html Maybe all caps? __MTFSB0/__MTFSB1 ? Of course, I don't think XL currently provides the __builtin versions, so the chances of an actual collision are vanishingly small. I would be nice to plan for better compatibility in the future, though. :-) PC
Em 07/02/2020 11:50, Paul Clarke escreveu: > On 2/7/20 8:34 AM, Rogerio Alves wrote: >> This patch refactor fenviline.h replaces some statements for >> builtins. >> >> --- >> >> * Changes in v2: Per Adhemerval review: don not redefine _buitins use a >> macro instead. Use of __builtin_popcount allows do eliminate the >> if (e == 0) test. __builtin_popcount(e) == 1 for e = 0 validate false >> while (e & (e - 1)) == 0 validate true if e = 0. Appended a comment >> about the weird asm constrain i#*X. The warning only appear at GCC >> versions 3 and below. If we even stops to support GCC 3 for installed >> headers we may replace that for plain `i`. >> sysdeps/powerpc/bits/fenvinline.h | 30 +++++++++++++++++------------- >> 1 file changed, 17 insertions(+), 13 deletions(-) >> >> diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h >> index 70689664e2..7b2b75e5ef 100644 >> --- a/sysdeps/powerpc/bits/fenvinline.h >> +++ b/sysdeps/powerpc/bits/fenvinline.h >> @@ -51,9 +51,19 @@ >> # define fegetround() __fegetround () >> >> # ifndef __NO_MATH_INLINES >> + >> +/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9. */ >> +# if !__GNUC_PREREQ(9, 0) >> /* 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'. */ >> + same as just plain 'i'. This warning only happens in old GCC >> + versions (gcc 3 or less). Otherwise plain 'i' works fine. */ >> +# define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i#*X" (__b)) >> +# define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i#*X" (__b)) >> +# else >> +# define __mtfsb0(__b) __builtin_mtfsb0 (__b) >> +# define __mtfsb1(__b) __builtin_mtfsb1 (__b) >> +# endif > > These names actually conflict with XL compiler builtins which serve the same purpose. https://www.ibm.com/support/knowledgecenter/SSXVZZ_16.1.1/com.ibm.xlcpp1611.lelinux.doc/compiler_ref/bifs_fpscr.html > > Maybe all caps? __MTFSB0/__MTFSB1 ? > Weird. I've tested with XL and it did not give any warning. But I was not aware about that. Thank you for point me that out. I will change it to a caps version. I guess it will be fine. > Of course, I don't think XL currently provides the __builtin versions, so the chances of an actual collision are vanishingly small. I would be nice to plan for better compatibility in the future, though. :-) > > PC >
diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h index 70689664e2..7b2b75e5ef 100644 --- a/sysdeps/powerpc/bits/fenvinline.h +++ b/sysdeps/powerpc/bits/fenvinline.h @@ -51,9 +51,19 @@ # define fegetround() __fegetround () # ifndef __NO_MATH_INLINES + +/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9. */ +# if !__GNUC_PREREQ(9, 0) /* 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'. */ + same as just plain 'i'. This warning only happens in old GCC + versions (gcc 3 or less). Otherwise plain 'i' works fine. */ +# define __mtfsb0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i#*X" (__b)) +# define __mtfsb1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i#*X" (__b)) +# else +# define __mtfsb0(__b) __builtin_mtfsb0 (__b) +# define __mtfsb1(__b) __builtin_mtfsb1 (__b) +# endif # if __GNUC_PREREQ(3, 4) @@ -61,15 +71,12 @@ # define feraiseexcept(__excepts) \ (__extension__ ({ \ int __e = __excepts; \ - int __ret; \ + int __ret = 0; \ 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))); \ - __ret = 0; \ + __mtfsb1 ((__builtin_clz (__e))); \ } \ else \ __ret = feraiseexcept (__e); \ @@ -80,15 +87,12 @@ # define feclearexcept(__excepts) \ (__extension__ ({ \ int __e = __excepts; \ - int __ret; \ + int __ret = 0; \ 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))); \ - __ret = 0; \ + __mtfsb0 ((__builtin_clz (__e))); \ } \ else \ __ret = feclearexcept (__e); \