This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] remove nested function hack_digit
- From: Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: Andreas Schwab <schwab at linux-m68k dot org>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 22 Sep 2014 14:57:27 -0700
- Subject: Re: [PATCH] remove nested function hack_digit
- Authentication-results: sourceware.org; auth=none
- References: <CAGQ9bdxUJaUzz=ndu-qnhkPGAH7=m5mFKxpDag=H693TeA2ORw at mail dot gmail dot com> <87a960l9ze dot fsf at igel dot home> <CAGQ9bdyxCW-_3rLy6uLg4Vc2FPx+gUL7PChaXA4i6aKmnjGVZg at mail dot gmail dot com> <mvm38bsyppg dot fsf at hawking dot suse dot de> <CAGQ9bdya8w_OmD=1wKayhLN51H+Jqaio3RGqtATKWc6_hPgBxQ at mail dot gmail dot com> <20140922214338 dot 0D30A2C3971 at topped-with-meat dot com>
On Mon, Sep 22, 2014 at 2:43 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> Are you asking about the results from my particular compiler (GCC
>> 4.8.2) or in theoretical sense?
>
> When we're concerned with generated code quality, in general it is
> sufficient to demonstrate the effects just with the most-recent or a
> pretty-recent GCC version. 4.8.2 is a fine reference version today.
>
>> It doesn't mean the new code is worse or better.
>
> What you showed does indeed mean that the new code is worse: more code to
> do the same thing. You did not show the code of hack_digit itself.
hack_digit becomes longer too due to longer function prologue epilogue:
Was:
000000000004a390 <hack_digit.13608>:
4a390: 55 push %rbp
4a391: 53 push %rbx
4a392: 4c 89 d3 mov %r10,%rbx
4a395: 48 83 ec 08 sub $0x8,%rsp
4a399: 41 8b 42 30 mov 0x30(%r10),%eax
4a39d: 85 c0 test %eax,%eax
Now:
000000000004a390 <hack_digit>:
4a390: 41 55 push %r13
4a392: 41 54 push %r12
4a394: 55 push %rbp
4a395: 4c 89 c5 mov %r8,%rbp
4a398: 4d 89 c8 mov %r9,%r8
4a39b: 53 push %rbx
4a39c: 48 89 cb mov %rcx,%rbx
4a39f: 48 83 ec 08 sub $0x8,%rsp
4a3a3: 85 ff test %edi,%edi
(similar for multiple epilogues)
> It's
> possible that things improved inside the function such that the total
> effect of your change is in fact an improvement or a wash, but you have to
> actually analyze all the code and make a real judgment to say one or the
> other.
>
>> There is no miracle in the nested function call,
>> you still need to pass all the parameters and this is just done with a
>> different calling convention.
>
> This is not really true, or at least presented in a rather misleading way.
> A call to a nested function does not pass "all the parameters" at all.
> Semantically, you could say that it "passes" them all by reference. But
> what it actually does is pass a single pointer parameter (the static chain)
> that is used inside the nested function for indirect access to the
> containing function's local variables. So the closest approximation of
> what the existing code is doing is to put all the referenced parent locals
> together into a local struct and pass a pointer to that.
I considered doing such a patch but it turned out a huge textual
change that will make the code much less readable.
Still, let me do it and send it here anyway, unless you tell me no to.
--kcc
>
> In this particular case, that might be worth doing or it might be just as
> good (or better) from a source maintenance perspective and/or a generated
> code quality perspective to pass individual variables by reference as your
> last patch does. The onus is on you to propose a particular change,
> demonstrate with thorough analysis that it improves or at least does not
> harm generated code quality, and make the aesthetic case that improves or
> does not substantially harm source maintenance.