Re: [Qemu-devel] [PATCH 09/10] ppc: Move exception generation code out of line

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3051 bytes --]

On Mon, Jun 13, 2016 at 10:36:24AM +0200, Cédric Le Goater wrote:
> On 06/13/2016 09:44 AM, Thomas Huth wrote:
> > On 13.06.2016 07:24, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >>
> >> There's no point inlining this, if you hit the exception case you exit
> >> anyway, and not inlining saves about 100K of code size (and cache
> >> footprint).
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> ---
> >>  target-ppc/translate.c | 9 ++++++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index f211d175c09c..600d5db2bb9a 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -283,7 +283,8 @@ void gen_update_current_nip(void *opaque)
> >>      tcg_gen_movi_tl(cpu_nip, ctx->nip);
> >>  }
> >>  
> >> -static inline void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
> >> +static void __attribute__((noinline))
> >> +gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
> >>  {
> >>      TCGv_i32 t0, t1;
> >>      if (ctx->exception == POWERPC_EXCP_NONE) {
> >> @@ -297,7 +298,8 @@ static inline void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t
> >>      ctx->exception = (excp);
> >>  }
> >>  
> >> -static inline void gen_exception(DisasContext *ctx, uint32_t excp)
> >> +static void __attribute__((noinline))
> >> +gen_exception(DisasContext *ctx, uint32_t excp)
> >>  {
> >>      TCGv_i32 t0;
> >>      if (ctx->exception == POWERPC_EXCP_NONE) {
> >> @@ -309,7 +311,8 @@ static inline void gen_exception(DisasContext *ctx, uint32_t excp)
> >>      ctx->exception = (excp);
> >>  }
> >>  
> >> -static inline void gen_debug_exception(DisasContext *ctx)
> >> +static void __attribute__((noinline))
> >> +gen_debug_exception(DisasContext *ctx)
> >>  {
> >>      TCGv_i32 t0;
> > 
> > Do you get the same results if you just remove the "inline" keyword,
> > without adding the "__attribute__((noinline))" ? If yes, I'd suggest to
> > do this patch without the "__attribute__((noinline))" - that's easier to
> > read, and the compiler can still decide to inline something in case it's
> > better one a certain architecture.
> 
> Yes. They are no differences. 
> 
> The interesting part though is that the .text is about the same size. 
> There is even a slight increase of ~2K with gcc 4.9.2 (intel host) and 
> a slight decrease of ~1K with gcc 5.3.1 (ppc64le host).
> 
> I guess we can just drop that patch. It does not seem to bring much.

I would prefer to see the inline keyword removed.  Except in the case
of tiny header functions, it's very rarely a good idea - usually the
compiler will have better information on whether to inline or not.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]