From: Cornelia Huck <cohuck@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Thomas Huth <thuth@redhat.com>,
qemu-s390x@nongnu.org, Cornelia Huck <cohuck@redhat.com>,
Richard Henderson <richard.henderson@linaro.org>,
qemu-devel@nongnu.org
Subject: [PULL 12/15] target/s390x: Move DisasFields into DisasContext
Date: Mon, 27 Jan 2020 13:20:13 +0100 [thread overview]
Message-ID: <20200127122016.18752-13-cohuck@redhat.com> (raw)
In-Reply-To: <20200127122016.18752-1-cohuck@redhat.com>
From: Richard Henderson <richard.henderson@linaro.org>
I believe that the separate allocation of DisasFields from DisasContext
was meant to limit the places from which we could access fields. But
that plan did not go unchanged, and since DisasContext contains a pointer
to fields, the substructure is accessible everywhere.
By allocating the substructure with DisasContext, we improve the locality
of the accesses by avoiding one level of pointer chasing. In addition,
we avoid a dangling pointer to stack allocated memory, diagnosed by static
checkers.
Launchpad: https://bugs.launchpad.net/bugs/1661815
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200123232248.1800-5-richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
target/s390x/translate.c | 22 +++++++++---------
target/s390x/translate_vx.inc.c | 40 ++++++++++++++++-----------------
2 files changed, 30 insertions(+), 32 deletions(-)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 51a1d865c04e..3674fee10c83 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -139,7 +139,7 @@ struct DisasFields {
struct DisasContext {
DisasContextBase base;
const DisasInsn *insn;
- DisasFields *fields;
+ DisasFields fields;
uint64_t ex_value;
/*
* During translate_one(), pc_tmp is used to determine the instruction
@@ -1094,14 +1094,14 @@ typedef enum {
static bool have_field1(const DisasContext *s, enum DisasFieldIndexO c)
{
- return (s->fields->presentO >> c) & 1;
+ return (s->fields.presentO >> c) & 1;
}
static int get_field1(const DisasContext *s, enum DisasFieldIndexO o,
enum DisasFieldIndexC c)
{
assert(have_field1(s, o));
- return s->fields->c[c];
+ return s->fields.c[c];
}
/* Describe the layout of each field in each format. */
@@ -3763,7 +3763,7 @@ static DisasJumpType op_risbg(DisasContext *s, DisasOps *o)
int pos, len, rot;
/* Adjust the arguments for the specific insn. */
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0x55: /* risbg */
case 0x59: /* risbgn */
i3 &= 63;
@@ -3804,7 +3804,7 @@ static DisasJumpType op_risbg(DisasContext *s, DisasOps *o)
len = i4 - i3 + 1;
pos = 63 - i4;
rot = i5 & 63;
- if (s->fields->op2 == 0x5d) {
+ if (s->fields.op2 == 0x5d) {
pos += 32;
}
@@ -3873,7 +3873,7 @@ static DisasJumpType op_rosbg(DisasContext *s, DisasOps *o)
tcg_gen_rotli_i64(o->in2, o->in2, i5);
/* Operate. */
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0x55: /* AND */
tcg_gen_ori_i64(o->in2, o->in2, ~mask);
tcg_gen_and_i64(o->out, o->out, o->in2);
@@ -4489,7 +4489,7 @@ static DisasJumpType op_stnosm(DisasContext *s, DisasOps *o)
tcg_gen_qemu_st8(t, o->addr1, get_mem_index(s));
tcg_temp_free_i64(t);
- if (s->fields->op == 0xac) {
+ if (s->fields.op == 0xac) {
tcg_gen_andi_i64(psw_mask, psw_mask,
(i2 << 56) | 0x00ffffffffffffffull);
} else {
@@ -6000,7 +6000,7 @@ static void in2_i2_32u_shl(DisasContext *s, DisasOps *o)
#ifndef CONFIG_USER_ONLY
static void in2_insn(DisasContext *s, DisasOps *o)
{
- o->in2 = tcg_const_i64(s->fields->raw_insn);
+ o->in2 = tcg_const_i64(s->fields.raw_insn);
}
#define SPEC_in2_insn 0
#endif
@@ -6299,15 +6299,13 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
{
const DisasInsn *insn;
DisasJumpType ret = DISAS_NEXT;
- DisasFields f;
DisasOps o = {};
/* Search for the insn in the table. */
- insn = extract_insn(env, s, &f);
+ insn = extract_insn(env, s, &s->fields);
/* Set up the strutures we use to communicate with the helpers. */
s->insn = insn;
- s->fields = &f;
/* Emit insn_start now that we know the ILEN. */
tcg_gen_insn_start(s->base.pc_next, s->cc_op, s->ilen);
@@ -6315,7 +6313,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
/* Not found means unimplemented/illegal opcode. */
if (insn == NULL) {
qemu_log_mask(LOG_UNIMP, "unimplemented opcode 0x%02x%02x\n",
- f.op, f.op2);
+ s->fields.op, s->fields.op2);
gen_illegal_opcode(s);
return DISAS_NORETURN;
}
diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index e1a2d25c2fa4..24558cce8070 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -732,7 +732,7 @@ static DisasJumpType op_vmr(DisasContext *s, DisasOps *o)
}
tmp = tcg_temp_new_i64();
- if (s->fields->op2 == 0x61) {
+ if (s->fields.op2 == 0x61) {
/* iterate backwards to avoid overwriting data we might need later */
for (dst_idx = NUM_VEC_ELEMENTS(es) - 1; dst_idx >= 0; dst_idx--) {
src_idx = dst_idx / 2;
@@ -796,7 +796,7 @@ static DisasJumpType op_vpk(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0x97:
if (get_field(s, m5) & 0x1) {
gen_gvec_3_ptr(v1, v2, v3, cpu_env, 0, vpks_cc[es - 1]);
@@ -1038,7 +1038,7 @@ static DisasJumpType op_vstl(DisasContext *s, DisasOps *o)
static DisasJumpType op_vup(DisasContext *s, DisasOps *o)
{
- const bool logical = s->fields->op2 == 0xd4 || s->fields->op2 == 0xd5;
+ const bool logical = s->fields.op2 == 0xd4 || s->fields.op2 == 0xd5;
const uint8_t v1 = get_field(s, v1);
const uint8_t v2 = get_field(s, v2);
const uint8_t src_es = get_field(s, m3);
@@ -1052,7 +1052,7 @@ static DisasJumpType op_vup(DisasContext *s, DisasOps *o)
}
tmp = tcg_temp_new_i64();
- if (s->fields->op2 == 0xd7 || s->fields->op2 == 0xd5) {
+ if (s->fields.op2 == 0xd7 || s->fields.op2 == 0xd5) {
/* iterate backwards to avoid overwriting data we might need later */
for (dst_idx = NUM_VEC_ELEMENTS(dst_es) - 1; dst_idx >= 0; dst_idx--) {
src_idx = dst_idx;
@@ -1389,7 +1389,7 @@ static DisasJumpType op_vec(DisasContext *s, DisasOps *o)
gen_program_exception(s, PGM_SPECIFICATION);
return DISAS_NORETURN;
}
- if (s->fields->op2 == 0xdb) {
+ if (s->fields.op2 == 0xdb) {
es |= MO_SIGN;
}
@@ -1567,7 +1567,7 @@ static DisasJumpType op_vmx(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xff:
gen_gvec_fn_3(smax, es, v1, v2, v3);
break;
@@ -1677,7 +1677,7 @@ static DisasJumpType op_vma(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xaa:
fn = &g_vmal[es];
break;
@@ -1764,7 +1764,7 @@ static DisasJumpType op_vm(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xa2:
gen_gvec_fn_3(mul, es, get_field(s, v1),
get_field(s, v2), get_field(s, v3));
@@ -1967,7 +1967,7 @@ static DisasJumpType op_vesv(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0x70:
gen_gvec_fn_3(shlv, es, v1, v2, v3);
break;
@@ -1998,7 +1998,7 @@ static DisasJumpType op_ves(DisasContext *s, DisasOps *o)
}
if (likely(!get_field(s, b2))) {
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0x30:
gen_gvec_fn_2i(shli, es, v1, v3, d2);
break;
@@ -2015,7 +2015,7 @@ static DisasJumpType op_ves(DisasContext *s, DisasOps *o)
shift = tcg_temp_new_i32();
tcg_gen_extrl_i64_i32(shift, o->addr1);
tcg_gen_andi_i32(shift, shift, NUM_VEC_ELEMENT_BITS(es) - 1);
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0x30:
gen_gvec_fn_2s(shls, es, v1, v3, shift);
break;
@@ -2038,7 +2038,7 @@ static DisasJumpType op_vsl(DisasContext *s, DisasOps *o)
TCGv_i64 shift = tcg_temp_new_i64();
read_vec_element_i64(shift, get_field(s, v3), 7, ES_8);
- if (s->fields->op2 == 0x74) {
+ if (s->fields.op2 == 0x74) {
tcg_gen_andi_i64(shift, shift, 0x7);
} else {
tcg_gen_andi_i64(shift, shift, 0x78);
@@ -2084,7 +2084,7 @@ static DisasJumpType op_vsra(DisasContext *s, DisasOps *o)
TCGv_i64 shift = tcg_temp_new_i64();
read_vec_element_i64(shift, get_field(s, v3), 7, ES_8);
- if (s->fields->op2 == 0x7e) {
+ if (s->fields.op2 == 0x7e) {
tcg_gen_andi_i64(shift, shift, 0x7);
} else {
tcg_gen_andi_i64(shift, shift, 0x78);
@@ -2101,7 +2101,7 @@ static DisasJumpType op_vsrl(DisasContext *s, DisasOps *o)
TCGv_i64 shift = tcg_temp_new_i64();
read_vec_element_i64(shift, get_field(s, v3), 7, ES_8);
- if (s->fields->op2 == 0x7c) {
+ if (s->fields.op2 == 0x7c) {
tcg_gen_andi_i64(shift, shift, 0x7);
} else {
tcg_gen_andi_i64(shift, shift, 0x78);
@@ -2524,7 +2524,7 @@ static DisasJumpType op_vfa(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xe3:
fn = se ? gen_helper_gvec_vfa64s : gen_helper_gvec_vfa64;
break;
@@ -2555,7 +2555,7 @@ static DisasJumpType op_wfc(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- if (s->fields->op2 == 0xcb) {
+ if (s->fields.op2 == 0xcb) {
gen_gvec_2_ptr(get_field(s, v1), get_field(s, v2),
cpu_env, 0, gen_helper_gvec_wfc64);
} else {
@@ -2581,7 +2581,7 @@ static DisasJumpType op_vfc(DisasContext *s, DisasOps *o)
}
if (cs) {
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xe8:
fn = se ? gen_helper_gvec_vfce64s_cc : gen_helper_gvec_vfce64_cc;
break;
@@ -2595,7 +2595,7 @@ static DisasJumpType op_vfc(DisasContext *s, DisasOps *o)
g_assert_not_reached();
}
} else {
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xe8:
fn = se ? gen_helper_gvec_vfce64s : gen_helper_gvec_vfce64;
break;
@@ -2630,7 +2630,7 @@ static DisasJumpType op_vcdg(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- switch (s->fields->op2) {
+ switch (s->fields.op2) {
case 0xc3:
fn = se ? gen_helper_gvec_vcdg64s : gen_helper_gvec_vcdg64;
break;
@@ -2688,7 +2688,7 @@ static DisasJumpType op_vfma(DisasContext *s, DisasOps *o)
return DISAS_NORETURN;
}
- if (s->fields->op2 == 0x8f) {
+ if (s->fields.op2 == 0x8f) {
fn = se ? gen_helper_gvec_vfma64s : gen_helper_gvec_vfma64;
} else {
fn = se ? gen_helper_gvec_vfms64s : gen_helper_gvec_vfms64;
--
2.21.1
next prev parent reply other threads:[~2020-01-27 12:27 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-27 12:20 [PULL 00/15] s390x update Cornelia Huck
2020-01-27 12:20 ` [PULL 01/15] s390x/sclp.c: remove unneeded label in sclp_service_call() Cornelia Huck
2020-01-27 12:20 ` [PULL 02/15] intc/s390_flic_kvm.c: remove unneeded label in kvm_flic_load() Cornelia Huck
2020-01-27 12:20 ` [PULL 03/15] s390x/event-facility.c: remove unneeded labels Cornelia Huck
2020-01-27 12:20 ` [PULL 04/15] s390x: adapter routes error handling Cornelia Huck
2020-01-27 12:20 ` [PULL 05/15] s390x/event-facility: fix error propagation Cornelia Huck
2020-01-27 12:20 ` [PULL 06/15] target/s390x: Remove duplicated ifdef macro Cornelia Huck
2020-01-27 12:20 ` [PULL 07/15] docs/devel: fix stable process doc formatting Cornelia Huck
2020-01-27 12:20 ` [PULL 08/15] target/s390x/kvm: Enable adapter interruption suppression again Cornelia Huck
2020-01-27 12:20 ` [PULL 09/15] target/s390x: Move struct DisasFields definition earlier Cornelia Huck
2020-01-27 12:20 ` [PULL 10/15] target/s390x: Remove DisasFields argument from callbacks Cornelia Huck
2020-01-27 12:20 ` [PULL 11/15] target/s390x: Pass DisasContext to get_field and have_field Cornelia Huck
2020-01-27 12:20 ` Cornelia Huck [this message]
2020-01-27 12:20 ` [PULL 13/15] target/s390x: Remove DisasFields argument from extract_insn Cornelia Huck
2020-01-27 12:20 ` [PULL 14/15] hw/s390x: Add a more verbose comment about get_machine_class() and the wrappers Cornelia Huck
2020-01-27 12:20 ` [PULL 15/15] s390x: sigp: Fix sense running reporting Cornelia Huck
2020-01-27 13:54 ` [PULL 00/15] s390x update Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200127122016.18752-13-cohuck@redhat.com \
--to=cohuck@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.