[1/2,powerpc] add 'volatile' to asm
Commit Message
From: "Paul A. Clarke" <pc@us.ibm.com>
Add 'volatile' keyword to a few asm statements, to force the compiler
to generate the instructions therein.
2019-06-12 Paul A. Clarke <pc@us.ibm.com>
* sysdeps/powerpc/fpu/fenv_libc.h (relax_fenv_state): Add 'volatile'.
* sysdeps/powerpc/fpu/fpu_control.h (__FPU_MFFS): Likewise.
(__FPU_MFFSL): Likewise.
(_FPU_SETCW): Likewise.
v2: This fixes issues seen by Tulio in my earlier posted patch
"[powerpc] fegetround: utilize faster method to get rounding mode"
which was not committed.
---
sysdeps/powerpc/fpu/fenv_libc.h | 4 ++--
sysdeps/powerpc/fpu_control.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
Comments
* Paul A. Clarke:
> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
> index f8dd1b7..f66bf24 100644
> --- a/sysdeps/powerpc/fpu/fenv_libc.h
> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
> @@ -56,9 +56,9 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
> #define relax_fenv_state() \
> do { \
> if (GLRO(dl_hwcap) & PPC_FEATURE_HAS_DFP) \
> - asm (".machine push; .machine \"power6\"; " \
> + asm volatile (".machine push; .machine \"power6\"; " \
> "mtfsfi 7,0,1; .machine pop"); \
> - asm ("mtfsfi 7,0"); \
> + asm volatile ("mtfsfi 7,0"); \
> } while(0)
I think this change is a no-op because asm statements without inputs and
outputs (“Basic Asm”) are implicitly volatile. Are you adding these
just for consistency?
Thanks,
Florian
On 6/12/19 1:55 PM, Florian Weimer wrote:
> * Paul A. Clarke:
>> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
>> index f8dd1b7..f66bf24 100644
>> --- a/sysdeps/powerpc/fpu/fenv_libc.h
>> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
>> @@ -56,9 +56,9 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
>> #define relax_fenv_state() \
>> do { \
>> if (GLRO(dl_hwcap) & PPC_FEATURE_HAS_DFP) \
>> - asm (".machine push; .machine \"power6\"; " \
>> + asm volatile (".machine push; .machine \"power6\"; " \
>> "mtfsfi 7,0,1; .machine pop"); \
>> - asm ("mtfsfi 7,0"); \
>> + asm volatile ("mtfsfi 7,0"); \
>> } while(0)
>
> I think this change is a no-op because asm statements without inputs and
> outputs (“Basic Asm”) are implicitly volatile. Are you adding these
> just for consistency?
Of course that's what I was doing! ;-)
Actually, no. What I was doing was making assumptions based on an incomplete understanding of the semantics to which you refer, but I like your explanation better.
I'm tempted to push those changes anyway, as I think there is less chance for confusion (like mine), unless there are objections.
PC
@@ -56,9 +56,9 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
#define relax_fenv_state() \
do { \
if (GLRO(dl_hwcap) & PPC_FEATURE_HAS_DFP) \
- asm (".machine push; .machine \"power6\"; " \
+ asm volatile (".machine push; .machine \"power6\"; " \
"mtfsfi 7,0,1; .machine pop"); \
- asm ("mtfsfi 7,0"); \
+ asm volatile ("mtfsfi 7,0"); \
} while(0)
/* Set/clear a particular FPSCR bit (for instance,
@@ -67,7 +67,7 @@ typedef unsigned int fpu_control_t;
/* Macros for accessing the hardware control word. */
# define __FPU_MFFS() \
({register double __fr; \
- __asm__ ("mffs %0" : "=f" (__fr)); \
+ __asm__ __volatile__("mffs %0" : "=f" (__fr)); \
__fr; \
})
@@ -81,7 +81,7 @@ typedef unsigned int fpu_control_t;
#ifdef _ARCH_PWR9
# define __FPU_MFFSL() \
({register double __fr; \
- __asm__ ("mffsl %0" : "=f" (__fr)); \
+ __asm__ __volatile__("mffsl %0" : "=f" (__fr)); \
__fr; \
})
#else
@@ -101,7 +101,7 @@ typedef unsigned int fpu_control_t;
__u.__ll = 0xfff80000LL << 32; /* This is a QNaN. */ \
__u.__ll |= (cw) & 0xffffffffLL; \
__fr = __u.__d; \
- __asm__ ("mtfsf 255,%0" : : "f" (__fr)); \
+ __asm__ __volatile__("mtfsf 255,%0" : : "f" (__fr)); \
}
/* Default control word set at startup. */