[Qemu-devel] [PATCH 0/5] SPARC: fix clang warnings

[Qemu-devel] [PATCH 0/5] SPARC: fix clang warnings

[Peter Maydell]

These patches fix warnings generated by clang. Patches 1-3
have been onlist before (and reviewed by RTH) but didn't get
applied I think because of a mixup between me and Mark about
which tree they should go in by. 4 and 5 are new.

Peter Maydell (5):
  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: Mark gen_load_trap_state_at_tl() as !CONFIG_USER_ONLY
  disas/sparc: Remove unused data sparc_opcode_archs[]

 disas/sparc.c              | 32 --------------------------------
 target-sparc/ldst_helper.c | 10 +++++-----
 target-sparc/translate.c   | 26 ++------------------------
 3 files changed, 7 insertions(+), 61 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/5] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 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 2/5] 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.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 target-sparc/ldst_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 1a62e19..42b80cb 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -250,6 +250,7 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,
 
 #endif
 
+#if defined(TARGET_SPARC64) || defined(CONFIG_USER_ONLY)
 static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
 {
 #ifdef TARGET_SPARC64
@@ -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,6 +289,7 @@ static inline int is_translating_asi(int asi)
 #endif
 }
 
+#ifdef TARGET_SPARC64
 static inline target_ulong asi_address_mask(CPUSPARCState *env,
                                             int asi, target_ulong 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 3/5] 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.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 target-sparc/ldst_helper.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 42b80cb..f27e0a6 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -262,11 +262,12 @@ static inline 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 */
+/* TODO: check sparc32 bits */
 static inline 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 inline target_ulong asi_address_mask(CPUSPARCState *env,
                                             int asi, target_ulong addr)
 {
-- 
1.9.1

[Qemu-devel] [PATCH 4/5] target-sparc: Mark gen_load_trap_state_at_tl() as !CONFIG_USER_ONLY

[Peter Maydell]

The function gen_load_trap_state_at_tl() is only used in the softmmu
configs; wrap it in #ifndef CONFIG_USER_ONLY to avoid clang compiler
warnings in linux-user builds.

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

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 62edf20..75d2317 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2300,6 +2300,7 @@ static void gen_fmovq(DisasContext *dc, DisasCompare *cmp, int rd, int rs)
     gen_update_fprs_dirty(qd);
 }
 
+#ifndef CONFIG_USER_ONLY
 static inline void gen_load_trap_state_at_tl(TCGv_ptr r_tsptr, TCGv_ptr cpu_env)
 {
     TCGv_i32 r_tl = tcg_temp_new_i32();
@@ -2324,6 +2325,7 @@ static inline void gen_load_trap_state_at_tl(TCGv_ptr r_tsptr, TCGv_ptr cpu_env)
 
     tcg_temp_free_i32(r_tl);
 }
+#endif
 
 static void gen_edge(DisasContext *dc, TCGv dst, TCGv s1, TCGv s2,
                      int width, bool cc, bool left)
-- 
1.9.1

[Qemu-devel] [PATCH 5/5] disas/sparc: Remove unused data sparc_opcode_archs[]

[Peter Maydell]

Remove sparc_opcode_archs and the macros which use it, because we don't
use them in QEMU and they provoke clang warnings:

disas/sparc.c:307:39: warning: unused variable 'sparc_opcode_archs' [-Wunused-const-variable]
static const struct sparc_opcode_arch sparc_opcode_archs[] =
                                      ^

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

diff --git a/disas/sparc.c b/disas/sparc.c
index 8e755d1..f4e3565 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -80,19 +80,6 @@ typedef struct sparc_opcode_arch
   short supported;
 } sparc_opcode_arch;
 
-static const struct sparc_opcode_arch sparc_opcode_archs[];
-
-/* Return the bitmask of supported architectures for ARCH.  */
-#define SPARC_OPCODE_SUPPORTED(ARCH) (sparc_opcode_archs[ARCH].supported)
-
-/* Non-zero if ARCH1 conflicts with ARCH2.
-   IE: ARCH1 as a supported bit set that ARCH2 doesn't, and vice versa.  */
-#define SPARC_OPCODE_CONFLICT_P(ARCH1, ARCH2) \
- (((SPARC_OPCODE_SUPPORTED (ARCH1) & SPARC_OPCODE_SUPPORTED (ARCH2)) \
-   != SPARC_OPCODE_SUPPORTED (ARCH1)) \
-  && ((SPARC_OPCODE_SUPPORTED (ARCH1) & SPARC_OPCODE_SUPPORTED (ARCH2)) \
-     != SPARC_OPCODE_SUPPORTED (ARCH2)))
-
 /* Structure of an opcode table entry.  */
 
 typedef struct sparc_opcode
@@ -301,25 +288,6 @@ static const char *sparc_decode_sparclet_cpreg (int);
    otherwise.  */
 #define v9notv9a        (MASK_V9)
 
-/* Table of opcode architectures.
-   The order is defined in opcode/sparc.h.  */
-
-static const struct sparc_opcode_arch sparc_opcode_archs[] =
-{
-  { "v6", MASK_V6 },
-  { "v7", MASK_V6 | MASK_V7 },
-  { "v8", MASK_V6 | MASK_V7 | MASK_V8 },
-  { "sparclet", MASK_V6 | MASK_V7 | MASK_V8 | MASK_SPARCLET },
-  { "sparclite", MASK_V6 | MASK_V7 | MASK_V8 | MASK_SPARCLITE },
-  /* ??? Don't some v8 privileged insns conflict with v9?  */
-  { "v9", MASK_V6 | MASK_V7 | MASK_V8 | MASK_V9 },
-  /* v9 with ultrasparc additions */
-  { "v9a", MASK_V6 | MASK_V7 | MASK_V8 | MASK_V9 | MASK_V9A },
-  /* v9 with cheetah additions */
-  { "v9b", MASK_V6 | MASK_V7 | MASK_V8 | MASK_V9 | MASK_V9A | MASK_V9B },
-  { NULL, 0 }
-};
-
 /* Branch condition field.  */
 #define COND(x)         (((x) & 0xf) << 25)
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/5] SPARC: fix clang warnings

[Mark Cave-Ayland]

On 23/12/14 22:11, Peter Maydell wrote:

> These patches fix warnings generated by clang. Patches 1-3
> have been onlist before (and reviewed by RTH) but didn't get
> applied I think because of a mixup between me and Mark about
> which tree they should go in by. 4 and 5 are new.
> 
> Peter Maydell (5):
>   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: Mark gen_load_trap_state_at_tl() as !CONFIG_USER_ONLY
>   disas/sparc: Remove unused data sparc_opcode_archs[]
> 
>  disas/sparc.c              | 32 --------------------------------
>  target-sparc/ldst_helper.c | 10 +++++-----
>  target-sparc/translate.c   | 26 ++------------------------
>  3 files changed, 7 insertions(+), 61 deletions(-)

Oh my apologies! Given that there's not much review that I can
personally do (and they are just compilation fixes) then I was expecting
these to go either via master or Richard. If that is still a problem let
me know and I'll queue them as part of my next pull request.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 0/5] SPARC: fix clang warnings

[Peter Maydell]

On 2 January 2015 at 13:57, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 23/12/14 22:11, Peter Maydell wrote:
>> These patches fix warnings generated by clang. Patches 1-3
>> have been onlist before (and reviewed by RTH) but didn't get
>> applied I think because of a mixup between me and Mark about
>> which tree they should go in by. 4 and 5 are new.

> Oh my apologies! Given that there's not much review that I can
> personally do (and they are just compilation fixes) then I was expecting
> these to go either via master or Richard. If that is still a problem let
> me know and I'll queue them as part of my next pull request.

Where we have a subsystem maintainer I tend to prefer to route
patches via them, on the assumption that they'll do a second
level of testing (even if only of the "yep, still boots"
variety). Also it avoids potential clashes between different
patches to the same target if they all go through your tree.
The idea behind the subsystem-maintainer setup is to spread
the workload a bit :-)

-- PMM

Re: [Qemu-devel] [PATCH 0/5] SPARC: fix clang warnings

[Mark Cave-Ayland]

On 02/01/15 17:30, Peter Maydell wrote:

> On 2 January 2015 at 13:57, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> On 23/12/14 22:11, Peter Maydell wrote:
>>> These patches fix warnings generated by clang. Patches 1-3
>>> have been onlist before (and reviewed by RTH) but didn't get
>>> applied I think because of a mixup between me and Mark about
>>> which tree they should go in by. 4 and 5 are new.
> 
>> Oh my apologies! Given that there's not much review that I can
>> personally do (and they are just compilation fixes) then I was expecting
>> these to go either via master or Richard. If that is still a problem let
>> me know and I'll queue them as part of my next pull request.
> 
> Where we have a subsystem maintainer I tend to prefer to route
> patches via them, on the assumption that they'll do a second
> level of testing (even if only of the "yep, still boots"
> variety). Also it avoids potential clashes between different
> patches to the same target if they all go through your tree.
> The idea behind the subsystem-maintainer setup is to spread
> the workload a bit :-)

I've finally managed to test these and they don't seem to break anything
in my boot tests so I've applied them to my qemu-sparc branch.

There is one more SPARC64 NVRAM patch I'd like to get in before sending
another pull request, however it depends upon on a related QEMU NVRAM
patch series which I'll send through to the list shortly.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 0/5] SPARC: fix clang warnings

[Mark Cave-Ayland]

On 18/01/15 21:15, Mark Cave-Ayland wrote:

> On 02/01/15 17:30, Peter Maydell wrote:
> 
>> On 2 January 2015 at 13:57, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>> On 23/12/14 22:11, Peter Maydell wrote:
>>>> These patches fix warnings generated by clang. Patches 1-3
>>>> have been onlist before (and reviewed by RTH) but didn't get
>>>> applied I think because of a mixup between me and Mark about
>>>> which tree they should go in by. 4 and 5 are new.
>>
>>> Oh my apologies! Given that there's not much review that I can
>>> personally do (and they are just compilation fixes) then I was expecting
>>> these to go either via master or Richard. If that is still a problem let
>>> me know and I'll queue them as part of my next pull request.
>>
>> Where we have a subsystem maintainer I tend to prefer to route
>> patches via them, on the assumption that they'll do a second
>> level of testing (even if only of the "yep, still boots"
>> variety). Also it avoids potential clashes between different
>> patches to the same target if they all go through your tree.
>> The idea behind the subsystem-maintainer setup is to spread
>> the workload a bit :-)
> 
> I've finally managed to test these and they don't seem to break anything
> in my boot tests so I've applied them to my qemu-sparc branch.
> 
> There is one more SPARC64 NVRAM patch I'd like to get in before sending
> another pull request, however it depends upon on a related QEMU NVRAM
> patch series which I'll send through to the list shortly.

Looks like the NVRAM patches I wanted to apply are now dependent upon
Herve's isa_mem_base removal and m48t59 QOM patches; I'll send through a
pull request with just your clang warning patchset in a moment to clear
them from the queue.


ATB,

Mark.