| Message ID | AANLkTimOkYH5r8jUdgC5xjnkaJjdMvFXrZMCcvPb3Vip@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Alexander Graf <agraf@suse.de> writes: > On 06.10.2010, at 23:34, Blue Swirl wrote: > >> Compiling with GCC 4.6.0 20100925 produced warnings: >> /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': >> /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but >> not used [-Werror=unused-but-set-variable] >> /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': >> /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set >> but not used [-Werror=unused-but-set-variable] >> /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': >> /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set >> but not used [-Werror=unused-but-set-variable] >> >> Fix by making the variable declarations and their uses also conditional >> to debug definition. Delete tmp. > > Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. Or simply (void)EPN.
On Wed, Oct 6, 2010 at 9:39 PM, Alexander Graf <agraf@suse.de> wrote: > > On 06.10.2010, at 23:34, Blue Swirl wrote: > >> Compiling with GCC 4.6.0 20100925 produced warnings: >> /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': >> /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but >> not used [-Werror=unused-but-set-variable] >> /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': >> /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set >> but not used [-Werror=unused-but-set-variable] >> /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': >> /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set >> but not used [-Werror=unused-but-set-variable] >> >> Fix by making the variable declarations and their uses also conditional >> to debug definition. Delete tmp. > > Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. Perhaps all conditionally enabled debug printf stuff should be transformed into tracepoints? That would be more flexible than uncommenting DEBUG_foobar. Would it help with bitrot too? > Signed-off-by: Alexander Graf <agraf@suse.de> Thanks.
Am 07.10.2010 um 20:59 schrieb Blue Swirl <blauwirbel@gmail.com>: > On Wed, Oct 6, 2010 at 9:39 PM, Alexander Graf <agraf@suse.de> wrote: >> >> On 06.10.2010, at 23:34, Blue Swirl wrote: >> >>> Compiling with GCC 4.6.0 20100925 produced warnings: >>> /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': >>> /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but >>> not used [-Werror=unused-but-set-variable] >>> /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': >>> /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set >>> but not used [-Werror=unused-but-set-variable] >>> /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': >>> /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set >>> but not used [-Werror=unused-but-set-variable] >>> >>> Fix by making the variable declarations and their uses also conditional >>> to debug definition. Delete tmp. >> >> Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. > > Perhaps all conditionally enabled debug printf stuff should be > transformed into tracepoints? That would be more flexible than > uncommenting DEBUG_foobar. Would it help with bitrot too? I haven't looked at the qemu tracepoints framework yet, but some of this debug stuff is in hot paths and only very rarely used. So unless it is overhead free, I'd prefer debug function stubs. Alex > >> Signed-off-by: Alexander Graf <agraf@suse.de> > > Thanks.
On Thu, Oct 7, 2010 at 8:08 PM, Alexander Graf <agraf@suse.de> wrote: > > Am 07.10.2010 um 20:59 schrieb Blue Swirl <blauwirbel@gmail.com>: > >> On Wed, Oct 6, 2010 at 9:39 PM, Alexander Graf <agraf@suse.de> wrote: >>> >>> On 06.10.2010, at 23:34, Blue Swirl wrote: >>> >>>> Compiling with GCC 4.6.0 20100925 produced warnings: >>>> /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': >>>> /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but >>>> not used [-Werror=unused-but-set-variable] >>>> /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': >>>> /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set >>>> but not used [-Werror=unused-but-set-variable] >>>> /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': >>>> /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set >>>> but not used [-Werror=unused-but-set-variable] >>>> >>>> Fix by making the variable declarations and their uses also conditional >>>> to debug definition. Delete tmp. >>> >>> Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. >> >> Perhaps all conditionally enabled debug printf stuff should be >> transformed into tracepoints? That would be more flexible than >> uncommenting DEBUG_foobar. Would it help with bitrot too? > > I haven't looked at the qemu tracepoints framework yet, but some of this debug stuff is in hot paths and only very rarely used. So unless it is overhead free, I'd prefer debug function stubs. There is zero overhead with 'nop' backend, with other backends there is some overhead. I think only 'nop' backend should be used on production versions, so this should be comparable to current debug stuff.
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c index 45f1655..348844f 100644 --- a/target-ppc/op_helper.c +++ b/target-ppc/op_helper.c @@ -348,15 +348,13 @@ void helper_dcbz_970(target_ulong addr) void helper_icbi(target_ulong addr) { - uint32_t tmp; - addr &= ~(env->dcache_line_size - 1); /* Invalidate one cache line : * PowerPC specification says this is to be treated like a load * (not a fetch) by the MMU. To be sure it will be so, * do the load "by hand". */ - tmp = ldl(addr); + ldl(addr); tb_invalidate_page_range(addr, addr + env->icache_line_size); } @@ -3802,16 +3800,23 @@ void helper_tlbie (target_ulong addr) /* PowerPC 602/603 software TLB load instructions helpers */ static void do_6xx_tlb (target_ulong new_EPN, int is_code) { - target_ulong RPN, CMP, EPN; + target_ulong RPN, CMP; +#ifdef DEBUG_SOFTWARE_TLB + target_ulong EPN; +#endif int way; RPN = env->spr[SPR_RPA]; if (is_code) { CMP = env->spr[SPR_ICMP]; +#ifdef DEBUG_SOFTWARE_TLB EPN = env->spr[SPR_IMISS]; +#endif } else { CMP = env->spr[SPR_DCMP]; +#ifdef DEBUG_SOFTWARE_TLB EPN = env->spr[SPR_DMISS]; +#endif } way = (env->spr[SPR_SRR1] >> 17) & 1; LOG_SWTLB("%s: EPN " TARGET_FMT_lx " " TARGET_FMT_lx " PTE0 " TARGET_FMT_lx @@ -3835,12 +3840,17 @@ void helper_6xx_tlbi (target_ulong EPN) /* PowerPC 74xx software TLB load instructions helpers */ static void do_74xx_tlb (target_ulong new_EPN, int is_code) { - target_ulong RPN, CMP, EPN; + target_ulong RPN, CMP; +#ifdef DEBUG_SOFTWARE_TLB + target_ulong EPN; +#endif int way; RPN = env->spr[SPR_PTELO]; CMP = env->spr[SPR_PTEHI]; +#ifdef DEBUG_SOFTWARE_TLB EPN = env->spr[SPR_TLBMISS] & ~0x3; +#endif way = env->spr[SPR_TLBMISS] & 0x3; LOG_SWTLB("%s: EPN " TARGET_FMT_lx " " TARGET_FMT_lx " PTE0 " TARGET_FMT_lx " PTE1 " TARGET_FMT_lx " way %d\n", __func__, new_EPN, EPN, CMP,
Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and their uses also conditional to debug definition. Delete tmp. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- target-ppc/op_helper.c | 20 +++++++++++++++----- 1 files changed, 15 insertions(+), 5 deletions(-)