* [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
@ 2015-05-12 20:20 Alexander Graf
2015-05-12 20:27 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2015-05-12 20:20 UTC (permalink / raw)
To: qemu-devel; +Cc: rth
We're currently missing the laa and laag instructions in our emulation.
In fact, we're missing the complete "interlocked-access facility 1" which
is part of zEC12. However, I really only needed the laa instruction for now.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
This really should implement all the other atomic load&modify instructions,
but I'd like to make sure we have a smart scheme to implement them first.
v1 -> v2:
- move atomic specific bits into load/store helpers, leave actual op
as normal op we can reuse
---
target-s390x/insn-data.def | 3 +++
target-s390x/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def
index 48e979e..cd95035 100644
--- a/target-s390x/insn-data.def
+++ b/target-s390x/insn-data.def
@@ -359,6 +359,9 @@
C(0xe371, LAY, RXY_a, LD, 0, a2, 0, r1, mov2, 0)
/* LOAD ADDRESS RELATIVE LONG */
C(0xc000, LARL, RIL_b, Z, 0, ri2, 0, r1, mov2, 0)
+/* LOAD AND ADD */
+ C(0xebf8, LAA, RSY_a, ILA, r3_32s, m2_32s_atomic, new, m2_32_r1_atomic, add, adds32)
+ C(0xebe8, LAAG, RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, add, adds64)
/* LOAD AND TEST */
C(0x1200, LTR, RR_a, Z, 0, r2_o, 0, cond_r1r2_32, mov2, s32)
C(0xb902, LTGR, RRE, Z, 0, r2_o, 0, r1, mov2, s64)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index fa3e334..73f2de3 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1118,6 +1118,7 @@ typedef enum DisasFacility {
FAC_PC, /* population count */
FAC_SCF, /* store clock fast */
FAC_SFLE, /* store facility list extended */
+ FAC_ILA, /* interlocked access facility 1 */
} DisasFacility;
struct DisasInsn {
@@ -1297,6 +1298,12 @@ static ExitStatus help_branch(DisasContext *s, DisasCompare *c,
return ret;
}
+static TCGv_i64 get_a2(DisasContext *s, DisasFields *f)
+{
+ int x2 = have_field(f, x2) ? get_field(f, x2) : 0;
+ return get_address(s, x2, get_field(f, b2), get_field(f, d2));
+}
+
/* ====================================================================== */
/* The operations. These perform the bulk of the work for any insn,
usually after the operands have been loaded and output initialized. */
@@ -4065,6 +4072,30 @@ static void wout_m2_32(DisasContext *s, DisasFields *f, DisasOps *o)
}
#define SPEC_wout_m2_32 0
+static void wout_m2_32_r1_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+ TCGv_i64 a2 = get_a2(s, f);
+
+ /* XXX release reservation */
+ tcg_gen_qemu_st32(o->out, a2, get_mem_index(s));
+ store_reg32_i64(get_field(f, r1), o->in2);
+
+ tcg_temp_free_i64(a2);
+}
+#define SPEC_wout_m2_32_r1_atomic 0
+
+static void wout_m2_64_r1_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+ TCGv_i64 a2 = get_a2(s, f);
+
+ /* XXX release reservation */
+ tcg_gen_qemu_st64(o->out, a2, get_mem_index(s));
+ store_reg(get_field(f, r1), o->in2);
+
+ tcg_temp_free_i64(a2);
+}
+#define SPEC_wout_m2_64_r1_atomic 0
+
/* ====================================================================== */
/* The "INput 1" generators. These load the first operand to an insn. */
@@ -4393,8 +4424,7 @@ static void in2_ra2(DisasContext *s, DisasFields *f, DisasOps *o)
static void in2_a2(DisasContext *s, DisasFields *f, DisasOps *o)
{
- int x2 = have_field(f, x2) ? get_field(f, x2) : 0;
- o->in2 = get_address(s, x2, get_field(f, b2), get_field(f, d2));
+ o->in2 = get_a2(s, f);
}
#define SPEC_in2_a2 0
@@ -4486,6 +4516,20 @@ static void in2_mri2_64(DisasContext *s, DisasFields *f, DisasOps *o)
}
#define SPEC_in2_mri2_64 0
+static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+ /* XXX should reserve the address */
+ in2_m2_32s(s, f, o);
+}
+#define SPEC_in2_m2_32s_atomic 0
+
+static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+ /* XXX should reserve the address */
+ in2_m2_64(s, f, o);
+}
+#define SPEC_in2_m2_64_atomic 0
+
static void in2_i2(DisasContext *s, DisasFields *f, DisasOps *o)
{
o->in2 = tcg_const_i64(get_field(f, i2));
--
1.7.12.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
2015-05-12 20:20 [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions Alexander Graf
@ 2015-05-12 20:27 ` Richard Henderson
2015-05-12 20:42 ` Alexander Graf
2015-05-12 20:46 ` Alexander Graf
0 siblings, 2 replies; 6+ messages in thread
From: Richard Henderson @ 2015-05-12 20:27 UTC (permalink / raw)
To: Alexander Graf, qemu-devel
On 05/12/2015 01:20 PM, Alexander Graf wrote:
> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
> +{
> + /* XXX should reserve the address */
> + in2_m2_32s(s, f, o);
> +}
> +#define SPEC_in2_m2_32s_atomic 0
> +
> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
> +{
> + /* XXX should reserve the address */
> + in2_m2_64(s, f, o);
> +}
> +#define SPEC_in2_m2_64_atomic 0
> +
I think these should save the address in o->addr1 so that you don't have to
recompute it in the wout functions.
Otherwise I think this is a great scheme, making it easy to implement this
whole family of instructions.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
2015-05-12 20:27 ` Richard Henderson
@ 2015-05-12 20:42 ` Alexander Graf
2015-05-12 21:15 ` Richard Henderson
2015-05-12 20:46 ` Alexander Graf
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2015-05-12 20:42 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 05/12/2015 10:27 PM, Richard Henderson wrote:
> On 05/12/2015 01:20 PM, Alexander Graf wrote:
>> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> + /* XXX should reserve the address */
>> + in2_m2_32s(s, f, o);
>> +}
>> +#define SPEC_in2_m2_32s_atomic 0
>> +
>> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> + /* XXX should reserve the address */
>> + in2_m2_64(s, f, o);
>> +}
>> +#define SPEC_in2_m2_64_atomic 0
>> +
> I think these should save the address in o->addr1 so that you don't have to
> recompute it in the wout functions.
But wouldn't it really be "addr2"? This is the address source for the
second argument after all.
Alex
>
> Otherwise I think this is a great scheme, making it easy to implement this
> whole family of instructions.
>
>
> r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
2015-05-12 20:27 ` Richard Henderson
2015-05-12 20:42 ` Alexander Graf
@ 2015-05-12 20:46 ` Alexander Graf
2015-05-12 21:19 ` Richard Henderson
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2015-05-12 20:46 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 05/12/2015 10:27 PM, Richard Henderson wrote:
> On 05/12/2015 01:20 PM, Alexander Graf wrote:
>> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> + /* XXX should reserve the address */
>> + in2_m2_32s(s, f, o);
>> +}
>> +#define SPEC_in2_m2_32s_atomic 0
>> +
>> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>> +{
>> + /* XXX should reserve the address */
>> + in2_m2_64(s, f, o);
>> +}
>> +#define SPEC_in2_m2_64_atomic 0
>> +
> I think these should save the address in o->addr1 so that you don't have to
> recompute it in the wout functions.
But I suppose you basically mean something like this?
Alex
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 73f2de3..8a30c8f 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -4074,25 +4074,17 @@ static void wout_m2_32(DisasContext *s,
DisasFields *f, DisasOps *o)
static void wout_m2_32_r1_atomic(DisasContext *s, DisasFields *f,
DisasOps *o)
{
- TCGv_i64 a2 = get_a2(s, f);
-
/* XXX release reservation */
- tcg_gen_qemu_st32(o->out, a2, get_mem_index(s));
+ tcg_gen_qemu_st32(o->out, o->addr1, get_mem_index(s));
store_reg32_i64(get_field(f, r1), o->in2);
-
- tcg_temp_free_i64(a2);
}
#define SPEC_wout_m2_32_r1_atomic 0
static void wout_m2_64_r1_atomic(DisasContext *s, DisasFields *f,
DisasOps *o)
{
- TCGv_i64 a2 = get_a2(s, f);
-
/* XXX release reservation */
- tcg_gen_qemu_st64(o->out, a2, get_mem_index(s));
+ tcg_gen_qemu_st64(o->out, o->addr1, get_mem_index(s));
store_reg(get_field(f, r1), o->in2);
-
- tcg_temp_free_i64(a2);
}
#define SPEC_wout_m2_64_r1_atomic 0
@@ -4519,14 +4511,18 @@ static void in2_mri2_64(DisasContext *s,
DisasFields *f, DisasOps *o)
static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f,
DisasOps *o)
{
/* XXX should reserve the address */
- in2_m2_32s(s, f, o);
+ o->addr1 = get_a2(s, f);
+ o->in2 = tcg_temp_new_i64();
+ tcg_gen_qemu_ld32s(o->in2, o->addr1, get_mem_index(s));
}
#define SPEC_in2_m2_32s_atomic 0
static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
{
/* XXX should reserve the address */
- in2_m2_64(s, f, o);
+ o->addr1 = get_a2(s, f);
+ o->in2 = tcg_temp_new_i64();
+ tcg_gen_qemu_ld64(o->in2, o->addr1, get_mem_index(s));
}
#define SPEC_in2_m2_64_atomic 0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
2015-05-12 20:42 ` Alexander Graf
@ 2015-05-12 21:15 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2015-05-12 21:15 UTC (permalink / raw)
To: Alexander Graf, qemu-devel
On 05/12/2015 01:42 PM, Alexander Graf wrote:
>
> But wouldn't it really be "addr2"? This is the address source for the second
> argument after all.
Yes, but we already abuse the name.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions
2015-05-12 20:46 ` Alexander Graf
@ 2015-05-12 21:19 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2015-05-12 21:19 UTC (permalink / raw)
To: Alexander Graf, qemu-devel
On 05/12/2015 01:46 PM, Alexander Graf wrote:
> On 05/12/2015 10:27 PM, Richard Henderson wrote:
>> On 05/12/2015 01:20 PM, Alexander Graf wrote:
>>> +static void in2_m2_32s_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>>> +{
>>> + /* XXX should reserve the address */
>>> + in2_m2_32s(s, f, o);
>>> +}
>>> +#define SPEC_in2_m2_32s_atomic 0
>>> +
>>> +static void in2_m2_64_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
>>> +{
>>> + /* XXX should reserve the address */
>>> + in2_m2_64(s, f, o);
>>> +}
>>> +#define SPEC_in2_m2_64_atomic 0
>>> +
>> I think these should save the address in o->addr1 so that you don't have to
>> recompute it in the wout functions.
>
> But I suppose you basically mean something like this?
Yes.
There's an in1_la2 that does what your get_a2 does.
Again, slightly misnamed, but that's the beauty of these atomic ops -- same
fields have gotten renamed.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-12 21:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 20:20 [Qemu-devel] [PATCH v2] s390x: Add laa and laag instructions Alexander Graf
2015-05-12 20:27 ` Richard Henderson
2015-05-12 20:42 ` Alexander Graf
2015-05-12 21:15 ` Richard Henderson
2015-05-12 20:46 ` Alexander Graf
2015-05-12 21:19 ` Richard Henderson
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.