mbox series

[0/8] tcg/s390 improvements

Message ID 20170829204759.6853-1-richard.henderson@linaro.org
Headers show
Series tcg/s390 improvements | expand

Message

Richard Henderson Aug. 29, 2017, 8:47 p.m. UTC
This finally converts s390 to the tcg_target_op_def function, which
allows constraints to vary at runtime.  Once we have that, we can

(1) Perform some facilities checks once during startup instead of
    every time we evaluate the constraint,
(2) Use the distinct-operands facility from z196.

In addition, two cleanups to the setcond expansion

(3) Use the load-on-condition-2 facility from z13,
(4) Use a smaller expansion for LE/LEU using SLBGR.

I wrote these after soft freeze and before I changed jobs, so it has
seen some testing.  Unfortunately, I no longer have access to s390
hardware, so I'd appreciate it if someone could re-test this rebase.


r~


Richard Henderson (8):
  tcg/s390: Fully convert tcg_target_op_def
  tcg/s390: Merge cmpi facilities check to tcg_target_op_def
  tcg/s390: Merge muli facilities check to tcg_target_op_def
  tcg/s390: Merge add2i facilities check to tcg_target_op_def
  tcg/s390: Merge ori+xori facilities check to tcg_target_op_def
  tcg/s390: Use distinct-operands facility
  tcg/s390: Use load-on-condition-2 facility
  tcg/s390: Use slbgr for setcond le and leu

 tcg/s390/tcg-target.h     |   2 +
 tcg/s390/tcg-target.inc.c | 700 +++++++++++++++++++++++++---------------------
 2 files changed, 380 insertions(+), 322 deletions(-)

-- 
2.13.5

Comments

no-reply@patchew.org Aug. 29, 2017, 9 p.m. UTC | #1
Hi,

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

Message-id: 20170829204759.6853-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH 0/8] tcg/s390 improvements
Type: series

=== 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
 t [tag update]            patchew/20170829122745.14309-1-stefanha@redhat.com -> patchew/20170829122745.14309-1-stefanha@redhat.com
 * [new tag]               patchew/20170829204759.6853-1-richard.henderson@linaro.org -> patchew/20170829204759.6853-1-richard.henderson@linaro.org
Switched to a new branch 'test'
efce52d267 tcg/s390: Use slbgr for setcond le and leu
35fae2aa5d tcg/s390: Use load-on-condition-2 facility
9ae14e93ab tcg/s390: Use distinct-operands facility
67d4b767e5 tcg/s390: Merge ori+xori facilities check to tcg_target_op_def
5a46cf248f tcg/s390: Merge add2i facilities check to tcg_target_op_def
f646646dd5 tcg/s390: Merge muli facilities check to tcg_target_op_def
c4698e87bb tcg/s390: Merge cmpi facilities check to tcg_target_op_def
4f75b31839 tcg/s390: Fully convert tcg_target_op_def

=== OUTPUT BEGIN ===
Checking PATCH 1/8: tcg/s390: Fully convert tcg_target_op_def...
Checking PATCH 2/8: tcg/s390: Merge cmpi facilities check to tcg_target_op_def...
ERROR: return is not a function, parentheses are not required
#104: FILE: tcg/s390/tcg-target.inc.c:2308:
+        return (s390_facilities & FACILITY_EXT_IMM ? &r_ri : &r_rZ);

ERROR: return is not a function, parentheses are not required
#107: FILE: tcg/s390/tcg-target.inc.c:2310:
+        return (s390_facilities & FACILITY_EXT_IMM ? &r_rC : &r_rZ);

ERROR: return is not a function, parentheses are not required
#122: FILE: tcg/s390/tcg-target.inc.c:2360:
+            return (s390_facilities & FACILITY_EXT_IMM ? &setc_c : &setc_z);

ERROR: return is not a function, parentheses are not required
#134: FILE: tcg/s390/tcg-target.inc.c:2370:
+            return (s390_facilities & FACILITY_EXT_IMM ? &movc_c : &movc_z);

total: 4 errors, 0 warnings, 118 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/8: tcg/s390: Merge muli facilities check to tcg_target_op_def...
ERROR: return is not a function, parentheses are not required
#87: FILE: tcg/s390/tcg-target.inc.c:2281:
+        return (s390_facilities & FACILITY_GEN_INST_EXT ? &r_0_ri : &r_0_rI);

ERROR: return is not a function, parentheses are not required
#90: FILE: tcg/s390/tcg-target.inc.c:2283:
+        return (s390_facilities & FACILITY_GEN_INST_EXT ? &r_0_rJ : &r_0_rI);

total: 2 errors, 0 warnings, 77 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 4/8: tcg/s390: Merge add2i facilities check to tcg_target_op_def...
ERROR: return is not a function, parentheses are not required
#84: FILE: tcg/s390/tcg-target.inc.c:2387:
+        return (s390_facilities & FACILITY_EXT_IMM ? &a2_ri : &a2_r);

ERROR: return is not a function, parentheses are not required
#92: FILE: tcg/s390/tcg-target.inc.c:2390:
+        return (s390_facilities & FACILITY_EXT_IMM ? &a2_rA : &a2_r);

total: 2 errors, 0 warnings, 77 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 5/8: tcg/s390: Merge ori+xori facilities check to tcg_target_op_def...
ERROR: return is not a function, parentheses are not required
#140: FILE: tcg/s390/tcg-target.inc.c:2229:
+        return (s390_facilities & FACILITY_EXT_IMM

ERROR: return is not a function, parentheses are not required
#145: FILE: tcg/s390/tcg-target.inc.c:2233:
+        return (s390_facilities & FACILITY_EXT_IMM

ERROR: return is not a function, parentheses are not required
#152: FILE: tcg/s390/tcg-target.inc.c:2240:
+        return (s390_facilities & FACILITY_EXT_IMM

ERROR: return is not a function, parentheses are not required
#157: FILE: tcg/s390/tcg-target.inc.c:2244:
+        return (s390_facilities & FACILITY_EXT_IMM

total: 4 errors, 0 warnings, 144 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 6/8: tcg/s390: Use distinct-operands facility...
ERROR: return is not a function, parentheses are not required
#251: FILE: tcg/s390/tcg-target.inc.c:2276:
+        return (s390_facilities & FACILITY_DISTINCT_OPS ? &r_r_ri : &r_0_ri);

ERROR: return is not a function, parentheses are not required
#284: FILE: tcg/s390/tcg-target.inc.c:2312:
+        return (s390_facilities & FACILITY_DISTINCT_OPS ? &r_r_ri : &r_0_ri);

ERROR: return is not a function, parentheses are not required
#290: FILE: tcg/s390/tcg-target.inc.c:2317:
+        return (s390_facilities & FACILITY_DISTINCT_OPS ? &r_r_ri : &r_0_ri);

total: 3 errors, 0 warnings, 263 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 7/8: tcg/s390: Use load-on-condition-2 facility...
ERROR: return is not a function, parentheses are not required
#188: FILE: tcg/s390/tcg-target.inc.c:2436:
+            return (s390_facilities & FACILITY_EXT_IMM

total: 1 errors, 0 warnings, 164 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 8/8: tcg/s390: Use slbgr for setcond le and leu...
=== 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
Cornelia Huck Aug. 30, 2017, 11:14 a.m. UTC | #2
On Tue, 29 Aug 2017 13:47:51 -0700
Richard Henderson <richard.henderson@linaro.org> wrote:

> This finally converts s390 to the tcg_target_op_def function, which

> allows constraints to vary at runtime.  Once we have that, we can

> 

> (1) Perform some facilities checks once during startup instead of

>     every time we evaluate the constraint,

> (2) Use the distinct-operands facility from z196.

> 

> In addition, two cleanups to the setcond expansion

> 

> (3) Use the load-on-condition-2 facility from z13,

> (4) Use a smaller expansion for LE/LEU using SLBGR.

> 

> I wrote these after soft freeze and before I changed jobs, so it has

> seen some testing.  Unfortunately, I no longer have access to s390

> hardware, so I'd appreciate it if someone could re-test this rebase.


FWIW, this passes my smoketests.

Looking at the patches now...
Cornelia Huck Aug. 30, 2017, 4:18 p.m. UTC | #3
On Wed, 30 Aug 2017 13:14:03 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 29 Aug 2017 13:47:51 -0700

> Richard Henderson <richard.henderson@linaro.org> wrote:

> 

> > This finally converts s390 to the tcg_target_op_def function, which

> > allows constraints to vary at runtime.  Once we have that, we can

> > 

> > (1) Perform some facilities checks once during startup instead of

> >     every time we evaluate the constraint,

> > (2) Use the distinct-operands facility from z196.

> > 

> > In addition, two cleanups to the setcond expansion

> > 

> > (3) Use the load-on-condition-2 facility from z13,

> > (4) Use a smaller expansion for LE/LEU using SLBGR.

> > 

> > I wrote these after soft freeze and before I changed jobs, so it has

> > seen some testing.  Unfortunately, I no longer have access to s390

> > hardware, so I'd appreciate it if someone could re-test this rebase.  

> 

> FWIW, this passes my smoketests.

> 

> Looking at the patches now...


That might have been a mistake :)

The parts that I managed to at least superficially understand looked
fine, though; so feel free to add

Acked-by: Cornelia Huck <cohuck@redhat.com>