[Qemu-devel] [PATCH v2 0/3] target-sparc: fixed unused function warnings

[Qemu-devel] [PATCH v2 0/3] target-sparc: fixed unused function warnings

[Peter Maydell]

These patchsets fix clang 3.4 warnings about unused static inline
functions (clang now warns about these if they're defined in a
.c file but then not used; gcc doesn't). The first patch just
removes two totally unused functions; the second two patches
use ifdeffery to avoid defining the functions in non-TARGET_SPARC64
builds.

(I sent out v1 back in June; the only change here is to drop
the now-unnecessary "inline" specifiers, as suggested by RTH.)

Peter Maydell (3):
  target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc
  target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64
    only
  target-sparc: is_translating_asi() is TARGET_SPARC64 only

 target-sparc/ldst_helper.c | 18 +++++++++---------
 target-sparc/translate.c   | 24 ------------------------
 2 files changed, 9 insertions(+), 33 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/3] target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc

[Peter Maydell]

The functions gen_op_addi_cc() and gen_op_subi_cc() are unused; remove them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-sparc/translate.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 78c4e21..62edf20 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -363,14 +363,6 @@ static inline void gen_mov_reg_C(TCGv reg, TCGv_i32 src)
     tcg_gen_andi_tl(reg, reg, 0x1);
 }
 
-static inline void gen_op_addi_cc(TCGv dst, TCGv src1, target_long src2)
-{
-    tcg_gen_mov_tl(cpu_cc_src, src1);
-    tcg_gen_movi_tl(cpu_cc_src2, src2);
-    tcg_gen_addi_tl(cpu_cc_dst, cpu_cc_src, src2);
-    tcg_gen_mov_tl(dst, cpu_cc_dst);
-}
-
 static inline void gen_op_add_cc(TCGv dst, TCGv src1, TCGv src2)
 {
     tcg_gen_mov_tl(cpu_cc_src, src1);
@@ -502,22 +494,6 @@ static void gen_op_addx_int(DisasContext *dc, TCGv dst, TCGv src1,
     }
 }
 
-static inline void gen_op_subi_cc(TCGv dst, TCGv src1, target_long src2, DisasContext *dc)
-{
-    tcg_gen_mov_tl(cpu_cc_src, src1);
-    tcg_gen_movi_tl(cpu_cc_src2, src2);
-    if (src2 == 0) {
-        tcg_gen_mov_tl(cpu_cc_dst, src1);
-        tcg_gen_movi_i32(cpu_cc_op, CC_OP_LOGIC);
-        dc->cc_op = CC_OP_LOGIC;
-    } else {
-        tcg_gen_subi_tl(cpu_cc_dst, cpu_cc_src, src2);
-        tcg_gen_movi_i32(cpu_cc_op, CC_OP_SUB);
-        dc->cc_op = CC_OP_SUB;
-    }
-    tcg_gen_mov_tl(dst, cpu_cc_dst);
-}
-
 static inline void gen_op_sub_cc(TCGv dst, TCGv src1, TCGv src2)
 {
     tcg_gen_mov_tl(cpu_cc_src, src1);
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/3] target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only

[Peter Maydell]

The address_mask() and asi_address_mask() functions are only used in
TARGET_SPARC64 configs, so guard with ifdefs to avoid warnings about
unused functions in 32-bit builds.

Since the main reason these functions were marked 'inline' was to
suppress unused-function warnings with gcc, we remove the 'inline'
as no longer necessary.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-sparc/ldst_helper.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 1a62e19..2d028a5 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -250,7 +250,8 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,
 
 #endif
 
-static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
+#if defined(TARGET_SPARC64) || defined(CONFIG_USER_ONLY)
+static target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
 {
 #ifdef TARGET_SPARC64
     if (AM_CHECK(env1)) {
@@ -259,6 +260,7 @@ static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
 #endif
     return addr;
 }
+#endif
 
 /* returns true if access using this ASI is to have address translated by MMU
    otherwise access is to raw physical address */
@@ -287,8 +289,9 @@ static inline int is_translating_asi(int asi)
 #endif
 }
 
-static inline target_ulong asi_address_mask(CPUSPARCState *env,
-                                            int asi, target_ulong addr)
+#ifdef TARGET_SPARC64
+static target_ulong asi_address_mask(CPUSPARCState *env,
+                                     int asi, target_ulong addr)
 {
     if (is_translating_asi(asi)) {
         return address_mask(env, addr);
@@ -296,6 +299,7 @@ static inline target_ulong asi_address_mask(CPUSPARCState *env,
         return addr;
     }
 }
+#endif
 
 void helper_check_align(CPUSPARCState *env, target_ulong addr, uint32_t align)
 {
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/3] target-sparc: is_translating_asi() is TARGET_SPARC64 only

[Peter Maydell]

Move the is_translating_asi() inside the TARGET_SPARC64 ifdef (and remove
the unimplemented 32-bit codepath), as it is only called from TARGET_SPARC64
code. This fixes a clang 3.4 unused-function warning.

Again, this allows us to remove an 'inline' tag that was there
largely to suppress gcc warnings.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-sparc/ldst_helper.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 2d028a5..9f65bc3 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -262,11 +262,12 @@ static target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
 }
 #endif
 
+#ifdef TARGET_SPARC64
 /* returns true if access using this ASI is to have address translated by MMU
    otherwise access is to raw physical address */
-static inline int is_translating_asi(int asi)
+/* TODO: check sparc32 bits */
+static int is_translating_asi(int asi)
 {
-#ifdef TARGET_SPARC64
     /* Ultrasparc IIi translating asi
        - note this list is defined by cpu implementation
     */
@@ -283,13 +284,8 @@ static inline int is_translating_asi(int asi)
     default:
         return 0;
     }
-#else
-    /* TODO: check sparc32 bits */
-    return 0;
-#endif
 }
 
-#ifdef TARGET_SPARC64
 static target_ulong asi_address_mask(CPUSPARCState *env,
                                      int asi, target_ulong addr)
 {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 0/3] target-sparc: fixed unused function warnings

[Richard Henderson]

On 09/02/2014 04:52 AM, Peter Maydell wrote:
> Peter Maydell (3):
>   target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc
>   target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64
>     only
>   target-sparc: is_translating_asi() is TARGET_SPARC64 only

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH v2 0/3] target-sparc: fixed unused function warnings

[Mark Cave-Ayland]

On 02/09/14 14:17, Richard Henderson wrote:

> On 09/02/2014 04:52 AM, Peter Maydell wrote:
>> Peter Maydell (3):
>>    target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc
>>    target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64
>>      only
>>    target-sparc: is_translating_asi() is TARGET_SPARC64 only

At first glance I can't see anything too controversial here, so I'm 
happy for this to be applied given it has passed Richard's review.


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 2/3] target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only

[Artyom Tarasenko]

On Tue, Sep 2, 2014 at 1:52 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The address_mask() and asi_address_mask() functions are only used in
> TARGET_SPARC64 configs, so guard with ifdefs to avoid warnings about
> unused functions in 32-bit builds.
>
> Since the main reason these functions were marked 'inline' was to
> suppress unused-function warnings with gcc, we remove the 'inline'
> as no longer necessary.

I thought address_mask was inline because it's in the hot path.
Wouldn't the removal hit performance?

Artyom

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-sparc/ldst_helper.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index 1a62e19..2d028a5 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -250,7 +250,8 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,
>
>  #endif
>
> -static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
> +#if defined(TARGET_SPARC64) || defined(CONFIG_USER_ONLY)
> +static target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
>  {
>  #ifdef TARGET_SPARC64
>      if (AM_CHECK(env1)) {
> @@ -259,6 +260,7 @@ static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
>  #endif
>      return addr;
>  }
> +#endif
>
>  /* returns true if access using this ASI is to have address translated by MMU
>     otherwise access is to raw physical address */
> @@ -287,8 +289,9 @@ static inline int is_translating_asi(int asi)
>  #endif
>  }
>
> -static inline target_ulong asi_address_mask(CPUSPARCState *env,
> -                                            int asi, target_ulong addr)
> +#ifdef TARGET_SPARC64
> +static target_ulong asi_address_mask(CPUSPARCState *env,
> +                                     int asi, target_ulong addr)
>  {
>      if (is_translating_asi(asi)) {
>          return address_mask(env, addr);
> @@ -296,6 +299,7 @@ static inline target_ulong asi_address_mask(CPUSPARCState *env,
>          return addr;
>      }
>  }
> +#endif
>
>  void helper_check_align(CPUSPARCState *env, target_ulong addr, uint32_t align)
>  {
> --
> 1.9.1
>
>



-- 
Regards,
Artyom Tarasenko

linux/sparc and solaris/sparc under qemu blog:
http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH v2 2/3] target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only

[Richard Henderson]

On 09/03/2014 01:49 AM, Artyom Tarasenko wrote:
> On Tue, Sep 2, 2014 at 1:52 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The address_mask() and asi_address_mask() functions are only used in
>> TARGET_SPARC64 configs, so guard with ifdefs to avoid warnings about
>> unused functions in 32-bit builds.
>>
>> Since the main reason these functions were marked 'inline' was to
>> suppress unused-function warnings with gcc, we remove the 'inline'
>> as no longer necessary.
> 
> I thought address_mask was inline because it's in the hot path.
> Wouldn't the removal hit performance?

The compiler will probably inline it itself as it sees fit.


r~