Skip to content

Commit

Permalink
target/s390x: Move DisasFields into DisasContext
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@rth7680 @cohuck
rth7680 authored and cohuck committed Jan 27, 2020
1 parent c30988d commit 344a7f6
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 32 deletions.
22 changes: 10 additions & 12 deletions target/s390x/translate.c
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -6299,23 +6299,21 @@ 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);

/* 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;
}
Expand Down
40 changes: 20 additions & 20 deletions target/s390x/translate_vx.inc.c
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 344a7f6

Please sign in to comment.