| Message ID | 1402160924-14152-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
07.06.2014 21:08, Peter Maydell wrote: > The tcg_out* and tcg_patch* functions are utility routines that may or > may not be used by a particular backend; mark them with the 'unused' > attribute to suppress spurious warnings if they aren't used. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > Do we want to define a QEMU_UNUSED macro in compiler.h? We don't > seem to have done for the existing uses of this attribute in tree. > > tcg/tcg.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index 2c5732d..fe55d05 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -125,19 +125,20 @@ static TCGRegSet tcg_target_available_regs[2]; > static TCGRegSet tcg_target_call_clobber_regs; > > #if TCG_TARGET_INSN_UNIT_SIZE == 1 > -static inline void tcg_out8(TCGContext *s, uint8_t v) Hm. Those are already #ifdef'ed, is it not enough? Which architectures do not use functions which _are_ declared/defined? But I wonder, why to bother at all? This "static inline foo() {..}" idiom is very common in header files, to define a function which may or may not be used, and no compiler warns about these. Thanks, /mjt
On 8 June 2014 13:11, Michael Tokarev <mjt@tls.msk.ru> wrote: > 07.06.2014 21:08, Peter Maydell wrote: >> The tcg_out* and tcg_patch* functions are utility routines that may or >> may not be used by a particular backend; mark them with the 'unused' >> attribute to suppress spurious warnings if they aren't used. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> Do we want to define a QEMU_UNUSED macro in compiler.h? We don't >> seem to have done for the existing uses of this attribute in tree. >> >> tcg/tcg.c | 20 ++++++++++++-------- >> 1 file changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index 2c5732d..fe55d05 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -125,19 +125,20 @@ static TCGRegSet tcg_target_available_regs[2]; >> static TCGRegSet tcg_target_call_clobber_regs; >> >> #if TCG_TARGET_INSN_UNIT_SIZE == 1 >> -static inline void tcg_out8(TCGContext *s, uint8_t v) > > Hm. Those are already #ifdef'ed, is it not enough? Which architectures > do not use functions which _are_ declared/defined? The tcg x86 backend doesn't use all the tcg_patch* functions. These ifdefs merely restrict the functions to only be provided for the backends which conceivably could use them; there is no requirement that the backend actually does use them. > But I wonder, why to bother at all? This "static inline foo() {..}" idiom > is very common in header files, to define a function which may or may not > be used, and no compiler warns about these. clang 3.4 complains, which is why I'm fixing these. (It also found about half a dozen genuine bugs and quite a bit of legitimately dead code, which is why I think it's worth dealing with the handful of false positives like this rather than removing the warning flag.) thanks -- PMM
On 8 June 2014 13:11, Michael Tokarev <mjt@tls.msk.ru> wrote: > 07.06.2014 21:08, Peter Maydell wrote: >> The tcg_out* and tcg_patch* functions are utility routines that may or >> may not be used by a particular backend; mark them with the 'unused' >> attribute to suppress spurious warnings if they aren't used. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> Do we want to define a QEMU_UNUSED macro in compiler.h? We don't >> seem to have done for the existing uses of this attribute in tree. >> >> tcg/tcg.c | 20 ++++++++++++-------- >> 1 file changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index 2c5732d..fe55d05 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -125,19 +125,20 @@ static TCGRegSet tcg_target_available_regs[2]; >> static TCGRegSet tcg_target_call_clobber_regs; >> >> #if TCG_TARGET_INSN_UNIT_SIZE == 1 >> -static inline void tcg_out8(TCGContext *s, uint8_t v) > > Hm. Those are already #ifdef'ed, is it not enough? Which architectures > do not use functions which _are_ declared/defined? > > But I wonder, why to bother at all? This "static inline foo() {..}" idiom > is very common in header files, to define a function which may or may not > be used, and no compiler warns about these. The other approach we could take would be to move these definitions from this .c file into a .h file (tcg-backend-helpers.h ?). Then clang won't complain (it seems to only warn about unused static-inline-functions in .c files, not in .h files). Richard: do you have a preference? thanks -- PMM
On 06/08/2014 12:04 PM, Peter Maydell wrote: > The other approach we could take would be to move these > definitions from this .c file into a .h file (tcg-backend-helpers.h ?). > Then clang won't complain (it seems to only warn about unused > static-inline-functions in .c files, not in .h files). > > Richard: do you have a preference? I don't like the idea of moving these to a public header, and I don't think it's worthwhile creating a new private header. I'm ok with the unused attribute. r~
07.06.2014 21:08, Peter Maydell wrote: > The tcg_out* and tcg_patch* functions are utility routines that may or > may not be used by a particular backend; mark them with the 'unused' > attribute to suppress spurious warnings if they aren't used. Applied to -trivial, thank you! /mjt
diff --git a/tcg/tcg.c b/tcg/tcg.c index 2c5732d..fe55d05 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -125,19 +125,20 @@ static TCGRegSet tcg_target_available_regs[2]; static TCGRegSet tcg_target_call_clobber_regs; #if TCG_TARGET_INSN_UNIT_SIZE == 1 -static inline void tcg_out8(TCGContext *s, uint8_t v) +static __attribute__((unused)) inline void tcg_out8(TCGContext *s, uint8_t v) { *s->code_ptr++ = v; } -static inline void tcg_patch8(tcg_insn_unit *p, uint8_t v) +static __attribute__((unused)) inline void tcg_patch8(tcg_insn_unit *p, + uint8_t v) { *p = v; } #endif #if TCG_TARGET_INSN_UNIT_SIZE <= 2 -static inline void tcg_out16(TCGContext *s, uint16_t v) +static __attribute__((unused)) inline void tcg_out16(TCGContext *s, uint16_t v) { if (TCG_TARGET_INSN_UNIT_SIZE == 2) { *s->code_ptr++ = v; @@ -148,7 +149,8 @@ static inline void tcg_out16(TCGContext *s, uint16_t v) } } -static inline void tcg_patch16(tcg_insn_unit *p, uint16_t v) +static __attribute__((unused)) inline void tcg_patch16(tcg_insn_unit *p, + uint16_t v) { if (TCG_TARGET_INSN_UNIT_SIZE == 2) { *p = v; @@ -159,7 +161,7 @@ static inline void tcg_patch16(tcg_insn_unit *p, uint16_t v) #endif #if TCG_TARGET_INSN_UNIT_SIZE <= 4 -static inline void tcg_out32(TCGContext *s, uint32_t v) +static __attribute__((unused)) inline void tcg_out32(TCGContext *s, uint32_t v) { if (TCG_TARGET_INSN_UNIT_SIZE == 4) { *s->code_ptr++ = v; @@ -170,7 +172,8 @@ static inline void tcg_out32(TCGContext *s, uint32_t v) } } -static inline void tcg_patch32(tcg_insn_unit *p, uint32_t v) +static __attribute__((unused)) inline void tcg_patch32(tcg_insn_unit *p, + uint32_t v) { if (TCG_TARGET_INSN_UNIT_SIZE == 4) { *p = v; @@ -181,7 +184,7 @@ static inline void tcg_patch32(tcg_insn_unit *p, uint32_t v) #endif #if TCG_TARGET_INSN_UNIT_SIZE <= 8 -static inline void tcg_out64(TCGContext *s, uint64_t v) +static __attribute__((unused)) inline void tcg_out64(TCGContext *s, uint64_t v) { if (TCG_TARGET_INSN_UNIT_SIZE == 8) { *s->code_ptr++ = v; @@ -192,7 +195,8 @@ static inline void tcg_out64(TCGContext *s, uint64_t v) } } -static inline void tcg_patch64(tcg_insn_unit *p, uint64_t v) +static __attribute__((unused)) inline void tcg_patch64(tcg_insn_unit *p, + uint64_t v) { if (TCG_TARGET_INSN_UNIT_SIZE == 8) { *p = v;
The tcg_out* and tcg_patch* functions are utility routines that may or may not be used by a particular backend; mark them with the 'unused' attribute to suppress spurious warnings if they aren't used. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Do we want to define a QEMU_UNUSED macro in compiler.h? We don't seem to have done for the existing uses of this attribute in tree. tcg/tcg.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)