1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 00/15] add disable-tcg option for x86 build
  4. View patch

[Qemu-devel] [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()

Yang Zhong posted 15 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()
Posted by Yang Zhong 8 years, 7 months ago 
Split the cpu_set_mxcsr()/cpu_set_fpuc() with specific tcg code.
tcg_update_mxcsr()/tcg_set_fpuc() need be implemented in tcg-stub.c
file if tcg is disabled.

Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 accel/stubs/tcg-stub.c   |  8 ++++++++
 target/i386/cpu.h        | 15 +++++++++++++--
 target/i386/fpu_helper.c |  8 +++-----
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index dafb1d0..91625a8 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -75,6 +75,14 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
 {
 }
 
+void tcg_update_mxcsr(CPUX86State *env)
+{
+}
+
+void tcg_set_fpuc(CPUX86State *env)
+{
+}
+
 void cpu_loop_exit(CPUState *cpu)
 {
     abort();
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 8b3b535..229b216 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1643,8 +1643,19 @@ static inline int32_t x86_get_a20_mask(CPUX86State *env)
 }
 
 /* fpu_helper.c */
-void cpu_set_mxcsr(CPUX86State *env, uint32_t val);
-void cpu_set_fpuc(CPUX86State *env, uint16_t val);
+void tcg_update_mxcsr(CPUX86State *env);
+void tcg_set_fpuc(CPUX86State *env);
+static inline void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
+{
+    env->mxcsr = mxcsr;
+    tcg_update_mxcsr(env);
+}
+
+static inline void cpu_set_fpuc(CPUX86State *env, uint16_t fpuc)
+{
+     env->fpuc = fpuc;
+     tcg_set_fpuc(env);
+}
 
 /* mem_helper.c */
 void helper_lock_init(void);
diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index 34fb5fc..a7550c9 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -1550,12 +1550,11 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
 #define SSE_RC_CHOP         0x6000
 #define SSE_FZ              0x8000
 
-void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
+void tcg_update_mxcsr(CPUX86State *env)
 {
+    uint32_t mxcsr = env->mxcsr;
     int rnd_type;
 
-    env->mxcsr = mxcsr;
-
     /* set rounding mode */
     switch (mxcsr & SSE_RC_MASK) {
     default:
@@ -1581,9 +1580,8 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
     set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status);
 }
 
-void cpu_set_fpuc(CPUX86State *env, uint16_t val)
+void tcg_set_fpuc(CPUX86State *env)
 {
-    env->fpuc = val;
     update_fp_status(env);
 }
 
-- 
1.9.1
Re: [Qemu-devel] [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()
Posted by Paolo Bonzini 8 years, 7 months ago 

On 21/06/2017 12:19, Yang Zhong wrote:
> Split the cpu_set_mxcsr()/cpu_set_fpuc() with specific tcg code.
> tcg_update_mxcsr()/tcg_set_fpuc() need be implemented in tcg-stub.c
> file if tcg is disabled.
> 
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> ---
>  accel/stubs/tcg-stub.c   |  8 ++++++++
>  target/i386/cpu.h        | 15 +++++++++++++--
>  target/i386/fpu_helper.c |  8 +++-----
>  3 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> index dafb1d0..91625a8 100644
> --- a/accel/stubs/tcg-stub.c
> +++ b/accel/stubs/tcg-stub.c
> @@ -75,6 +75,14 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
>  {
>  }
>  
> +void tcg_update_mxcsr(CPUX86State *env)
> +{
> +}
> +
> +void tcg_set_fpuc(CPUX86State *env)
> +{
> +}
> +
>  void cpu_loop_exit(CPUState *cpu)
>  {
>      abort();
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 8b3b535..229b216 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1643,8 +1643,19 @@ static inline int32_t x86_get_a20_mask(CPUX86State *env)
>  }
>  
>  /* fpu_helper.c */
> -void cpu_set_mxcsr(CPUX86State *env, uint32_t val);
> -void cpu_set_fpuc(CPUX86State *env, uint16_t val);
> +void tcg_update_mxcsr(CPUX86State *env);
> +void tcg_set_fpuc(CPUX86State *env);
> +static inline void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
> +{
> +    env->mxcsr = mxcsr;
> +    tcg_update_mxcsr(env);

Instead of having to add stubs, please guard the call with "if
(tcg_enabled())".  Same below.

Paolo

> +}
> +
> +static inline void cpu_set_fpuc(CPUX86State *env, uint16_t fpuc)
> +{
> +     env->fpuc = fpuc;
> +     tcg_set_fpuc(env);
> +}
>  
>  /* mem_helper.c */
>  void helper_lock_init(void);
> diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
> index 34fb5fc..a7550c9 100644
> --- a/target/i386/fpu_helper.c
> +++ b/target/i386/fpu_helper.c
> @@ -1550,12 +1550,11 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
>  #define SSE_RC_CHOP         0x6000
>  #define SSE_FZ              0x8000
>  
> -void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
> +void tcg_update_mxcsr(CPUX86State *env)
>  {
> +    uint32_t mxcsr = env->mxcsr;
>      int rnd_type;
>  
> -    env->mxcsr = mxcsr;
> -
>      /* set rounding mode */
>      switch (mxcsr & SSE_RC_MASK) {
>      default:
> @@ -1581,9 +1580,8 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
>      set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status);
>  }
>  
> -void cpu_set_fpuc(CPUX86State *env, uint16_t val)
> +void tcg_set_fpuc(CPUX86State *env)
>  {
> -    env->fpuc = val;
>      update_fp_status(env);
>  }
>  
>
Re: [Qemu-devel] [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()
Posted by Zhong Yang 8 years, 7 months ago 
On Wed, Jun 21, 2017 at 03:15:25PM +0200, Paolo Bonzini wrote:
> 
> 
> On 21/06/2017 12:19, Yang Zhong wrote:
> > Split the cpu_set_mxcsr()/cpu_set_fpuc() with specific tcg code.
> > tcg_update_mxcsr()/tcg_set_fpuc() need be implemented in tcg-stub.c
> > file if tcg is disabled.
> > 
> > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > ---
> >  accel/stubs/tcg-stub.c   |  8 ++++++++
> >  target/i386/cpu.h        | 15 +++++++++++++--
> >  target/i386/fpu_helper.c |  8 +++-----
> >  3 files changed, 24 insertions(+), 7 deletions(-)
> > 
> > diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> > index dafb1d0..91625a8 100644
> > --- a/accel/stubs/tcg-stub.c
> > +++ b/accel/stubs/tcg-stub.c
> > @@ -75,6 +75,14 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
> >  {
> >  }
> >  
> > +void tcg_update_mxcsr(CPUX86State *env)
> > +{
> > +}
> > +
> > +void tcg_set_fpuc(CPUX86State *env)
> > +{
> > +}
> > +
> >  void cpu_loop_exit(CPUState *cpu)
> >  {
> >      abort();
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 8b3b535..229b216 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1643,8 +1643,19 @@ static inline int32_t x86_get_a20_mask(CPUX86State *env)
> >  }
> >  
> >  /* fpu_helper.c */
> > -void cpu_set_mxcsr(CPUX86State *env, uint32_t val);
> > -void cpu_set_fpuc(CPUX86State *env, uint16_t val);
> > +void tcg_update_mxcsr(CPUX86State *env);
> > +void tcg_set_fpuc(CPUX86State *env);
> > +static inline void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
> > +{
> > +    env->mxcsr = mxcsr;
> > +    tcg_update_mxcsr(env);
> 
> Instead of having to add stubs, please guard the call with "if
> (tcg_enabled())".  Same below.
> 
> Paolo
>
  Hello Paolo,

  Got it! thanks!

  From your comments + Richard Henderson's comments, i got below guideline
  (1) accel/stubs/tcg-stub.c
      This file is target-independent file, do not add x86-specific stubs functions into this file.

  (2) there are three kind of methods to mask TCG relative function
      a) if(tcg_enabled())
      b) if CONFIG_TCG
      c) stub function

  Paolo, which one is your prefer?  a) ?  please also make sure your prefer sequence, a) > b) > c) ?
  

  Yang
  
 
> > +}
> > +
> > +static inline void cpu_set_fpuc(CPUX86State *env, uint16_t fpuc)
> > +{
> > +     env->fpuc = fpuc;
> > +     tcg_set_fpuc(env);
> > +}
> >  
> >  /* mem_helper.c */
> >  void helper_lock_init(void);
> > diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
> > index 34fb5fc..a7550c9 100644
> > --- a/target/i386/fpu_helper.c
> > +++ b/target/i386/fpu_helper.c
> > @@ -1550,12 +1550,11 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
> >  #define SSE_RC_CHOP         0x6000
> >  #define SSE_FZ              0x8000
> >  
> > -void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
> > +void tcg_update_mxcsr(CPUX86State *env)
> >  {
> > +    uint32_t mxcsr = env->mxcsr;
> >      int rnd_type;
> >  
> > -    env->mxcsr = mxcsr;
> > -
> >      /* set rounding mode */
> >      switch (mxcsr & SSE_RC_MASK) {
> >      default:
> > @@ -1581,9 +1580,8 @@ void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
> >      set_flush_to_zero((mxcsr & SSE_FZ) ? 1 : 0, &env->fp_status);
> >  }
> >  
> > -void cpu_set_fpuc(CPUX86State *env, uint16_t val)
> > +void tcg_set_fpuc(CPUX86State *env)
> >  {
> > -    env->fpuc = val;
> >      update_fp_status(env);
> >  }
> >  
> >
Re: [Qemu-devel] [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()
Posted by Paolo Bonzini 8 years, 7 months ago 

----- Original Message -----
> From: "Zhong Yang" <yang.zhong@intel.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: qemu-devel@nongnu.org, "anthony xu" <anthony.xu@intel.com>, "a rigo" <a.rigo@virtualopensystems.com>, "Richard
> Henderson" <rth@twiddle.net>, "Thomas Huth" <thuth@redhat.com>
> Sent: Thursday, June 22, 2017 10:03:25 AM
> Subject: Re: [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()
> 
> On Wed, Jun 21, 2017 at 03:15:25PM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 21/06/2017 12:19, Yang Zhong wrote:
> > > Split the cpu_set_mxcsr()/cpu_set_fpuc() with specific tcg code.
> > > tcg_update_mxcsr()/tcg_set_fpuc() need be implemented in tcg-stub.c
> > > file if tcg is disabled.
> > > 
> > > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > > ---
> > >  accel/stubs/tcg-stub.c   |  8 ++++++++
> > >  target/i386/cpu.h        | 15 +++++++++++++--
> > >  target/i386/fpu_helper.c |  8 +++-----
> > >  3 files changed, 24 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> > > index dafb1d0..91625a8 100644
> > > --- a/accel/stubs/tcg-stub.c
> > > +++ b/accel/stubs/tcg-stub.c
> > > @@ -75,6 +75,14 @@ void dump_opcount_info(FILE *f, fprintf_function
> > > cpu_fprintf)
> > >  {
> > >  }
> > >  
> > > +void tcg_update_mxcsr(CPUX86State *env)
> > > +{
> > > +}
> > > +
> > > +void tcg_set_fpuc(CPUX86State *env)
> > > +{
> > > +}
> > > +
> > >  void cpu_loop_exit(CPUState *cpu)
> > >  {
> > >      abort();
> > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > index 8b3b535..229b216 100644
> > > --- a/target/i386/cpu.h
> > > +++ b/target/i386/cpu.h
> > > @@ -1643,8 +1643,19 @@ static inline int32_t x86_get_a20_mask(CPUX86State
> > > *env)
> > >  }
> > >  
> > >  /* fpu_helper.c */
> > > -void cpu_set_mxcsr(CPUX86State *env, uint32_t val);
> > > -void cpu_set_fpuc(CPUX86State *env, uint16_t val);
> > > +void tcg_update_mxcsr(CPUX86State *env);
> > > +void tcg_set_fpuc(CPUX86State *env);
> > > +static inline void cpu_set_mxcsr(CPUX86State *env, uint32_t mxcsr)
> > > +{
> > > +    env->mxcsr = mxcsr;
> > > +    tcg_update_mxcsr(env);
> > 
> > Instead of having to add stubs, please guard the call with "if
> > (tcg_enabled())".  Same below.
> > 
> > Paolo
> >
>   Hello Paolo,
> 
>   Got it! thanks!
> 
>   From your comments + Richard Henderson's comments, i got below guideline
>   (1) accel/stubs/tcg-stub.c
>       This file is target-independent file, do not add x86-specific stubs
>       functions into this file.
> 
>   (2) there are three kind of methods to mask TCG relative function
>       a) if(tcg_enabled())
>       b) if CONFIG_TCG
>       c) stub function
> 
>   Paolo, which one is your prefer?  a) ?  please also make sure your prefer
>   sequence, a) > b) > c) ?

There is also

   d) method in AccelClass

It seldom makes sense, but when it does it's the best.  Apart from this one,
a) > c) > b).

For target-independent code, "c" is more often the solution.  For target-dependent
code, "a" is more common.

Paolo
Re: [Qemu-devel] [PATCH 11/15] tcg: split cpu_set_mxcsr()/cpu_set_fpuc()
Posted by Richard Henderson 8 years, 7 months ago 
On 06/21/2017 03:19 AM, Yang Zhong wrote:
> Split the cpu_set_mxcsr()/cpu_set_fpuc() with specific tcg code.
> tcg_update_mxcsr()/tcg_set_fpuc() need be implemented in tcg-stub.c
> file if tcg is disabled.
> 
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> ---
>   accel/stubs/tcg-stub.c   |  8 ++++++++
>   target/i386/cpu.h        | 15 +++++++++++++--
>   target/i386/fpu_helper.c |  8 +++-----
>   3 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> index dafb1d0..91625a8 100644
> --- a/accel/stubs/tcg-stub.c
> +++ b/accel/stubs/tcg-stub.c
> @@ -75,6 +75,14 @@ void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
>   {
>   }
>   
> +void tcg_update_mxcsr(CPUX86State *env)
> +{
> +}
> +
> +void tcg_set_fpuc(CPUX86State *env)
> +{
> +}

Do not put x86-specific functions in what is clearly a target-independent file.
Paolo gave you one way of doing that for this case.


r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.