Re: [PATCH] remove nested function hack_digit

[Konstantin Serebryany <konstantin dot s dot serebryany at gmail 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.