There are two conceptually unrelated cleanups in here, though
the second touches many of the same lines as the first, so
separating the two would be ugly.

The first is to split gen_opparam_buf and move the pieces into
TCGOp.  This has two effects: the operands for an op is in the
same cacheline as the op, and we get to drop the pointer into
gen_opparam_buf, freeing up a register and/or function argument.

The second is to change what value is stored in TCGArg for each
TCG temporary.  Rather than store the index into tcg_ctx.temps,
store the pointer to the temp itself.  This allows us to drop
some arithmetic on many uses of a temp within the backend.

Making that second change is tricky, as we don't want to miss any
of the places that ought to be changed.  To do that I introduce a
number of helpers.

As a final step I changed the type of TCGOp.args to a structure,
and annotated the places that access constant arguments.  I found
that final patch to be really ugly, so I dropped it.  But I'm
fairly confident that I've updated all of the non-constant args.

The effect of this is nearly noise, but does reduce code size,

   text	   data	    bss	    dec	    hex	filename
6648688	2106408	4486112	13241208 ca0b78	qemu-system-alpha (before)
6627656	2106408	4502496	13236560 c9f950	qemu-system-alpha (after)

or about 21k.


r~


Richard Henderson (16):
  tcg: Merge opcode arguments into TCGOp
  tcg: Propagate args to op->args in optimizer
  tcg: Propagate args to op->args in tcg.c
  tcg: Propagate TCGOp down to allocators
  tcg: Introduce arg_temp
  tcg: Add temp_global bit to TCGTemp
  tcg: Return NULL temp for TCG_CALL_DUMMY_ARG
  tcg: Introduce temp_arg
  tcg: Use per-temp state data in liveness
  tcg: Avoid loops against variable bounds
  tcg: Change temp_allocate_frame arg to TCGTemp
  tcg: Remove unused TCG_CALL_DUMMY_TCGV
  tcg: Export temp_idx
  tcg: Use per-temp state data in optimize
  tcg: Define separate structures for TCGv_*
  tcg: Store pointers to temporaries directly in TCGArg

 tcg/optimize.c | 647 ++++++++++++++++++++++++++++++++-------------------------
 tcg/tcg-op.c   |  99 ++++-----
 tcg/tcg.c      | 610 ++++++++++++++++++++++++-----------------------------
 tcg/tcg.h      | 287 ++++++++++++++-----------
 4 files changed, 841 insertions(+), 802 deletions(-)

-- 
2.9.4

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 00/16] Cleanups within TCG middle-end
Type: series
Message-id: 20170621024831.26019-1-rth@twiddle.net

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1498014889-52658-1-git-send-email-wanpeng.li@hotmail.com -> patchew/1498014889-52658-1-git-send-email-wanpeng.li@hotmail.com
Switched to a new branch 'test'
4a3cb84 tcg: Store pointers to temporaries directly in TCGArg
02ed29c tcg: Define separate structures for TCGv_*
5cf0662 tcg: Use per-temp state data in optimize
5fdbde2 tcg: Export temp_idx
15a10ce tcg: Remove unused TCG_CALL_DUMMY_TCGV
4fa8938 tcg: Change temp_allocate_frame arg to TCGTemp
4c11a05 tcg: Avoid loops against variable bounds
6976828 tcg: Use per-temp state data in liveness
1f69381 tcg: Introduce temp_arg
af94763 tcg: Return NULL temp for TCG_CALL_DUMMY_ARG
cb3e123 tcg: Add temp_global bit to TCGTemp
11681e4 tcg: Introduce arg_temp
0d81916 tcg: Propagate TCGOp down to allocators
b2dc5b3 tcg: Propagate args to op->args in tcg.c
d095548 tcg: Propagate args to op->args in optimizer
33e16df tcg: Merge opcode arguments into TCGOp

=== OUTPUT BEGIN ===
Checking PATCH 1/16: tcg: Merge opcode arguments into TCGOp...
ERROR: spaces prohibited around that ':' (ctx:WxW)
#480: FILE: tcg/tcg.h:619:
+    unsigned calli  : 4;        /* 12 */
                     ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#481: FILE: tcg/tcg.h:620:
+    unsigned callo  : 2;        /* 14 */
                     ^

ERROR: space prohibited before that ':' (ctx:WxW)
#482: FILE: tcg/tcg.h:621:
+    unsigned        : 2;        /* 16 */
                     ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#487: FILE: tcg/tcg.h:624:
+    unsigned prev   : 16;       /* 32 */
                     ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#488: FILE: tcg/tcg.h:625:
+    unsigned next   : 16;       /* 48 */
                     ^

total: 5 errors, 0 warnings, 481 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/16: tcg: Propagate args to op->args in optimizer...
ERROR: spaces required around that '-' (ctx:VxV)
#644: FILE: tcg/optimize.c:1165:
+                tcg_opt_gen_mov(s, op, op->args[0], op->args[4-tmp]);
                                                               ^

total: 1 errors, 0 warnings, 912 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/16: tcg: Propagate args to op->args in tcg.c...
Checking PATCH 4/16: tcg: Propagate TCGOp down to allocators...
Checking PATCH 5/16: tcg: Introduce arg_temp...
Checking PATCH 6/16: tcg: Add temp_global bit to TCGTemp...
Checking PATCH 7/16: tcg: Return NULL temp for TCG_CALL_DUMMY_ARG...
Checking PATCH 8/16: tcg: Introduce temp_arg...
Checking PATCH 9/16: tcg: Use per-temp state data in liveness...
WARNING: line over 80 characters
#186: FILE: tcg/tcg.c:1572:
+            } else if (arg_temp(op->args[0])->state == TS_DEAD && have_opc_new2) {

total: 0 errors, 1 warnings, 442 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 10/16: tcg: Avoid loops against variable bounds...
Checking PATCH 11/16: tcg: Change temp_allocate_frame arg to TCGTemp...
Checking PATCH 12/16: tcg: Remove unused TCG_CALL_DUMMY_TCGV...
Checking PATCH 13/16: tcg: Export temp_idx...
Checking PATCH 14/16: tcg: Use per-temp state data in optimize...
Checking PATCH 15/16: tcg: Define separate structures for TCGv_*...
Checking PATCH 16/16: tcg: Store pointers to temporaries directly in TCGArg...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Richard Henderson <rth@twiddle.net> writes:

> There are two conceptually unrelated cleanups in here, though
> the second touches many of the same lines as the first, so
> separating the two would be ugly.
>
> The first is to split gen_opparam_buf and move the pieces into
> TCGOp.  This has two effects: the operands for an op is in the
> same cacheline as the op, and we get to drop the pointer into
> gen_opparam_buf, freeing up a register and/or function argument.
>
> The second is to change what value is stored in TCGArg for each
> TCG temporary.  Rather than store the index into tcg_ctx.temps,
> store the pointer to the temp itself.  This allows us to drop
> some arithmetic on many uses of a temp within the backend.
>
> Making that second change is tricky, as we don't want to miss any
> of the places that ought to be changed.  To do that I introduce a
> number of helpers.
>
> As a final step I changed the type of TCGOp.args to a structure,
> and annotated the places that access constant arguments.  I found
> that final patch to be really ugly, so I dropped it.  But I'm
> fairly confident that I've updated all of the non-constant args.
>
> The effect of this is nearly noise, but does reduce code size,
>
>    text	   data	    bss	    dec	    hex	filename
> 6648688	2106408	4486112	13241208 ca0b78	qemu-system-alpha (before)
> 6627656	2106408	4502496	13236560 c9f950	qemu-system-alpha (after)
>
> or about 21k.

Hmm it compile tested fine on mine but:

qemu-system-sparc: /home/alex/lsrc/qemu/qemu.git/tcg/tcg.h:725: temp_arg: Assertion `n < tcg_ctx.nb_temps' failed.
Broken pipe
GTester: last random seed: R02Sd6911d835e4140adeb8780ec1bf70af1
qemu-system-sparc: /home/alex/lsrc/qemu/qemu.git/tcg/tcg.h:725: temp_arg: Assertion `n < tcg_ctx.nb_temps' failed.
Broken pipe
GTester: last random seed: R02Sff8343267ed0c224ea97a4f54e970d65
qemu-system-sparc: /home/alex/lsrc/qemu/qemu.git/tcg/tcg.h:725: temp_arg: Assertion `n < tcg_ctx.nb_temps' failed.
Broken pipe
GTester: last random seed: R02Sfb0d300ff314d4f31a4f5bb414f5f249
/home/alex/lsrc/qemu/qemu.git/tests/Makefile.include:824: recipe for target 'check-qtest-sparc' failed
make: *** [check-qtest-sparc] Error 1
test: Expected a combining operator like '-a' at index 1

And also Travis is quite un-happy:

  https://travis-ci.org/stsquad/qemu/builds/247099991


>
>
> r~
>
>
> Richard Henderson (16):
>   tcg: Merge opcode arguments into TCGOp
>   tcg: Propagate args to op->args in optimizer
>   tcg: Propagate args to op->args in tcg.c
>   tcg: Propagate TCGOp down to allocators
>   tcg: Introduce arg_temp
>   tcg: Add temp_global bit to TCGTemp
>   tcg: Return NULL temp for TCG_CALL_DUMMY_ARG
>   tcg: Introduce temp_arg
>   tcg: Use per-temp state data in liveness
>   tcg: Avoid loops against variable bounds
>   tcg: Change temp_allocate_frame arg to TCGTemp
>   tcg: Remove unused TCG_CALL_DUMMY_TCGV
>   tcg: Export temp_idx
>   tcg: Use per-temp state data in optimize
>   tcg: Define separate structures for TCGv_*
>   tcg: Store pointers to temporaries directly in TCGArg
>
>  tcg/optimize.c | 647 ++++++++++++++++++++++++++++++++-------------------------
>  tcg/tcg-op.c   |  99 ++++-----
>  tcg/tcg.c      | 610 ++++++++++++++++++++++++-----------------------------
>  tcg/tcg.h      | 287 ++++++++++++++-----------
>  4 files changed, 841 insertions(+), 802 deletions(-)


--
Alex Bennée

On 06/26/2017 09:49 AM, Alex Bennée wrote:
> Hmm it compile tested fine on mine but:
> 
> qemu-system-sparc: /home/alex/lsrc/qemu/qemu.git/tcg/tcg.h:725: temp_arg: Assertion `n < tcg_ctx.nb_temps' failed.

Bah.  I fixed this, and then lost it while rebasing.
It's an uninitialized structure member.

> And also Travis is quite un-happy:
> 
>    https://travis-ci.org/stsquad/qemu/builds/247099991

The only real failure I see in here is also GTESTER for sparc.
Or am I missing something?


r~

Richard Henderson <rth@twiddle.net> writes:

> On 06/26/2017 09:49 AM, Alex Bennée wrote:
>> Hmm it compile tested fine on mine but:
>>
>> qemu-system-sparc: /home/alex/lsrc/qemu/qemu.git/tcg/tcg.h:725: temp_arg: Assertion `n < tcg_ctx.nb_temps' failed.
>
> Bah.  I fixed this, and then lost it while rebasing.
> It's an uninitialized structure member.
>
>> And also Travis is quite un-happy:
>>
>>    https://travis-ci.org/stsquad/qemu/builds/247099991
>
> The only real failure I see in here is also GTESTER for sparc.
> Or am I missing something?

There is also a compile failure on Travis:

  https://travis-ci.org/stsquad/qemu/jobs/247099992#L3021
  https://travis-ci.org/stsquad/qemu/jobs/247099995#L3020

I hadn't actually clicked through all the failures because I assumed
they are mostly the same. I'll re-check on v2 ;-)

>
>
> r~


--
Alex Bennée