[Qemu-devel] [PATCH 00/13] target-arm: A64 decoder set 2: misc logic and bit ops

[Qemu-devel] [PATCH 00/13] target-arm: A64 decoder set 2: misc logic and bit ops

[Peter Maydell]

The first chunk of A64 decoder patches have now made it through
code review, so as promised here's the next chunk. This is a
grabbag of miscellaneous logic and bit-twiddling operations,
plus some other minor stuff like ADR and conditional-select.

(Set three is probably going to be loads and stores. Set four
is likely to be 'everything else needed to run a simple binary':
add/sub, multiplies, the user-space visible MRS/MSR registers, svc,
fp<->integer register moves and the exclusive load/store group,
though that might get split into 'four' and 'five' sets if it's
not all cooked by the time 'three' gets through patch review.
Following that in some order should be VFP, Neon, and anything
that fell through the cracks (adc, anybody?).)

Git tree (with v7-cpu-host/mach-virt, v8 kvm control,
 and A64 set one all underneath these ptaches):
 git://git.linaro.org/people/pmaydell/qemu-arm.git a64-second-set
web UI:
 https://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/a64-second-set


Alexander Graf (7):
  target-arm: A64: add support for logical (shifted register)
  target-arm: A64: add support for ADR and ADRP
  target-arm: A64: add support for EXTR
  target-arm: A64: add support for 2-src data processing and DIV
  target-arm: A64: add support for 2-src shift reg insns
  target-arm: A64: add support for 1-src RBIT insn
  target-arm: A64: add support for logical (immediate) insns

Claudio Fontana (6):
  target-arm: A64: add support for conditional select
  target-arm: A64: add support for 1-src data processing and CLZ
  target-arm: A64: add support for 1-src REV insns
  target-arm: A64: add support for bitfield insns
  host-utils: add clrsb32/64 - count leading redundant sign bits
  target-arm: A64: add support for 1-src CLS insn

 include/qemu/host-utils.h  |   32 ++
 target-arm/helper-a64.c    |   54 +++
 target-arm/helper-a64.h    |    6 +
 target-arm/translate-a64.c |  783 ++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 855 insertions(+), 20 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

This patch adds support for the instruction group "C3.5.6
Conditional select": CSEL, CSINC, CSINV, CSNEG.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   56 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index fdc3ed8..a9c1803 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -724,10 +724,62 @@ static void disas_cc_reg(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Conditional select */
+/* C3.5.6 Conditional select
+ *   31   30  29  28             21 20  16 15  12 11 10 9    5 4    0
+ * +----+----+---+-----------------+------+------+-----+------+------+
+ * | sf | op | S | 1 1 0 1 0 1 0 0 |  Rm  | cond | op2 |  Rn  |  Rd  |
+ * +----+----+---+-----------------+------+------+-----+------+------+
+ */
 static void disas_cond_select(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, else_inv, rm, cond, else_inc, rn, rd;
+    TCGv_i64 tcg_rd, tcg_src;
+
+    if (extract32(insn, 29, 1) || extract32(insn, 11, 1)) {
+        /* S == 1 or op2<1> == 1 */
+        unallocated_encoding(s);
+        return;
+    }
+    sf = extract32(insn, 31, 1);
+    else_inv = extract32(insn, 30, 1);
+    rm = extract32(insn, 16, 5);
+    cond = extract32(insn, 12, 4);
+    else_inc = extract32(insn, 10, 1);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+    tcg_rd = cpu_reg(s, rd);
+
+    if (cond >= 0x0e) { /* condition "always" */
+        tcg_src = read_cpu_reg(s, rn, sf);
+        tcg_gen_mov_i64(tcg_rd, tcg_src);
+    } else {
+        /* OPTME: we could use movcond here, at the cost of duplicating
+         * a lot of the arm_gen_test_cc() logic.
+         */
+        int label_match = gen_new_label();
+        int label_continue = gen_new_label();
+
+        arm_gen_test_cc(cond, label_match);
+        /* nomatch: */
+        tcg_src = read_cpu_reg(s, rm, sf);
+        tcg_gen_mov_i64(tcg_rd, tcg_src);
+        if (else_inv) {
+            tcg_gen_not_i64(tcg_rd, tcg_rd);
+        }
+        if (else_inc) {
+            tcg_gen_addi_i64(tcg_rd, tcg_rd, 1);
+        }
+        if (!sf) {
+            tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+        }
+        tcg_gen_br(label_continue);
+        /* match: */
+        gen_set_label(label_match);
+        tcg_src = read_cpu_reg(s, rn, sf);
+        tcg_gen_mov_i64(tcg_rd, tcg_src);
+        /* continue: */
+        gen_set_label(label_continue);
+    }
 }
 
 /* Data-processing (1 source) */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> +    if (cond >= 0x0e) { /* condition "always" */
> +        tcg_src = read_cpu_reg(s, rn, sf);
> +        tcg_gen_mov_i64(tcg_rd, tcg_src);

I wonder if it's worth adding that 0x0[ef] case to the generic condition
processing rather than keep replicating it everywhere.

> +    } else {
> +        /* OPTME: we could use movcond here, at the cost of duplicating
> +         * a lot of the arm_gen_test_cc() logic.
> +         */

Honestly, arm_gen_test_cc should get refactored to a real test (as opposed to
branch) sooner rather than later.

Longer term it's probably worth recognizing the special case of Rm==31 &&
Rn==31 && else_inc as setcond as opposed to movcond.

> +        arm_gen_test_cc(cond, label_match);
> +        /* nomatch: */
> +        tcg_src = read_cpu_reg(s, rm, sf);
> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
> +        if (else_inv) {
> +            tcg_gen_not_i64(tcg_rd, tcg_rd);
> +        }
> +        if (else_inc) {
> +            tcg_gen_addi_i64(tcg_rd, tcg_rd, 1);
> +        }

I think better as

  if (else_inv && else_inc) {
      tcg_gen_neg_i64(tcg_rd, tcg_src);
  } else if (else_inv) {
      tcg_gen_not_i64(tcg_rd, tcg_src);
  } else if (else_inc) {
      tcg_gen_addi_i64(tcg_rd, tcg_src, 1);
  } else {
      tcg_gen_mov_i64(tcg_rd, tcg_src);
  }

> +        if (!sf) {
> +            tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
> +        }

I do wonder about the usefulness of passing SF (as opposed to hardcoding 1) to
read_cpu_reg to begin, since the ext32u that it generates is redundant with the
one here at the end, and likely cannot be optimized away.


r~

Re: [Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Peter Maydell]

On 5 December 2013 22:26, Richard Henderson <rth@twiddle.net> wrote:
> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>> +    if (cond >= 0x0e) { /* condition "always" */
>> +        tcg_src = read_cpu_reg(s, rn, sf);
>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>
> I wonder if it's worth adding that 0x0[ef] case to the generic condition
> processing rather than keep replicating it everywhere.
>
>> +    } else {
>> +        /* OPTME: we could use movcond here, at the cost of duplicating
>> +         * a lot of the arm_gen_test_cc() logic.
>> +         */
>
> Honestly, arm_gen_test_cc should get refactored to a real test (as opposed to
> branch) sooner rather than later.

By "sooner rather than later" do you mean "as part of this patch series" ?

> Longer term it's probably worth recognizing the special case of Rm==31 &&
> Rn==31 && else_inc as setcond as opposed to movcond.
>
>> +        arm_gen_test_cc(cond, label_match);
>> +        /* nomatch: */
>> +        tcg_src = read_cpu_reg(s, rm, sf);
>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>> +        if (else_inv) {
>> +            tcg_gen_not_i64(tcg_rd, tcg_rd);
>> +        }
>> +        if (else_inc) {
>> +            tcg_gen_addi_i64(tcg_rd, tcg_rd, 1);
>> +        }
>
> I think better as
>
>   if (else_inv && else_inc) {
>       tcg_gen_neg_i64(tcg_rd, tcg_src);
>   } else if (else_inv) {
>       tcg_gen_not_i64(tcg_rd, tcg_src);
>   } else if (else_inc) {
>       tcg_gen_addi_i64(tcg_rd, tcg_src, 1);
>   } else {
>       tcg_gen_mov_i64(tcg_rd, tcg_src);
>   }
>
>> +        if (!sf) {
>> +            tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
>> +        }
>
> I do wonder about the usefulness of passing SF (as opposed to hardcoding 1) to
> read_cpu_reg to begin, since the ext32u that it generates is redundant with the
> one here at the end, and likely cannot be optimized away.

Mmm. I did think that was a little unfortunate but didn't think of the
idea of just passing 1 to read_cpu_reg() for some reason.

-- PMM

Re: [Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Richard Henderson]

On 12/06/2013 11:31 AM, Peter Maydell wrote:
> On 5 December 2013 22:26, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>>> +    if (cond >= 0x0e) { /* condition "always" */
>>> +        tcg_src = read_cpu_reg(s, rn, sf);
>>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>>
>> I wonder if it's worth adding that 0x0[ef] case to the generic condition
>> processing rather than keep replicating it everywhere.
>>
>>> +    } else {
>>> +        /* OPTME: we could use movcond here, at the cost of duplicating
>>> +         * a lot of the arm_gen_test_cc() logic.
>>> +         */
>>
>> Honestly, arm_gen_test_cc should get refactored to a real test (as opposed to
>> branch) sooner rather than later.
> 
> By "sooner rather than later" do you mean "as part of this patch series" ?

It might make later patch series easier.  But I won't insist.


r~

Re: [Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Peter Maydell]

On 5 December 2013 22:26, Richard Henderson <rth@twiddle.net> wrote:
> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>> +    if (cond >= 0x0e) { /* condition "always" */
>> +        tcg_src = read_cpu_reg(s, rn, sf);
>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>
> I wonder if it's worth adding that 0x0[ef] case to the generic condition
> processing rather than keep replicating it everywhere.

I think "always true" is a special case anyway because you don't
want to emit any kind of branching/label logic at all.

>> +    } else {
>> +        /* OPTME: we could use movcond here, at the cost of duplicating
>> +         * a lot of the arm_gen_test_cc() logic.
>> +         */
>
> Honestly, arm_gen_test_cc should get refactored to a real test (as opposed to
> branch) sooner rather than later.

I had a think about this and I couldn't really come up with a particularly
nice looking API for it, given the way that movcondi/setcondi/brcondi work.
The best I could come up with was something like:

enum ARMCondType {
     CondSimple;
     CondAnd;
     CondOr;
};
typedef struct ARMCondState {
     int type;
     TCGv_i32 cmpop;
     TCGCond cond;
     bool cmpop_is_temp;
} ARMCondState;

/**
 * arm_cond_gen_test_cc:
 * @condstate: filled in with information about condition to check
 * @armcc: ARM condition value
 * @round: which of the (max 2) tests to deal with
 *
 * Decode the ARM condition value and identify which TCG condition
 * and operation to use. May emit code to set up the condition arguments.
 * The general idea is that you call with:
 *   arm_test_gen_cc(&condstate, armcc, 1);
 * and then emit a brcondi/movcondi/setcondi against zero with the
 * filled in condstate.cmpop and condstate.cond. If condstate.cmpop_is_temp
 * is true you then have to free cmpop with tcg_temp_free_i32().
 * If the condstate.type is CondAnd or CondOr then this is a condition
 * which is in two parts (eg gt: which is !Z && N == V). In this case then
 * (a) you are expected to set up multiple labels, feed movcond output
 * into a second movcond, etc to achieve the AND/OR effect and (b)
 * you should call the function a second time with round == 2 to get the
 * information for the second branch/comparison/test.
 */
void arm_gen_test_cc(ARMCondState *condstate, int armcc, int round);

But that seems pretty ugly to me.

The other awkwardness is that there's no easy way to do a "conditional
move of 64 bit values based on 32 bit comparison", which means you need
to sign extend the condstate.cmpop for 64 bit versions.

So I definitely think I'd rather postpone this for now, unless you have
a neat idea that I've missed for making it look nice.

>> +        arm_gen_test_cc(cond, label_match);
>> +        /* nomatch: */
>> +        tcg_src = read_cpu_reg(s, rm, sf);
>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>> +        if (else_inv) {
>> +            tcg_gen_not_i64(tcg_rd, tcg_rd);
>> +        }
>> +        if (else_inc) {
>> +            tcg_gen_addi_i64(tcg_rd, tcg_rd, 1);
>> +        }
>
> I think better as
>
>   if (else_inv && else_inc) {
>       tcg_gen_neg_i64(tcg_rd, tcg_src);
>   } else if (else_inv) {
>       tcg_gen_not_i64(tcg_rd, tcg_src);
>   } else if (else_inc) {
>       tcg_gen_addi_i64(tcg_rd, tcg_src, 1);
>   } else {
>       tcg_gen_mov_i64(tcg_rd, tcg_src);
>   }

Agreed, will fix.

>> +        if (!sf) {
>> +            tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
>> +        }
>
> I do wonder about the usefulness of passing SF (as opposed to hardcoding 1) to
> read_cpu_reg to begin, since the ext32u that it generates is redundant with the
> one here at the end, and likely cannot be optimized away.

Will fix.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Richard Henderson]

On 12/07/2013 01:45 AM, Peter Maydell wrote:
> On 5 December 2013 22:26, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>>> +    if (cond >= 0x0e) { /* condition "always" */
>>> +        tcg_src = read_cpu_reg(s, rn, sf);
>>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>>
>> I wonder if it's worth adding that 0x0[ef] case to the generic condition
>> processing rather than keep replicating it everywhere.
> 
> I think "always true" is a special case anyway because you don't
> want to emit any kind of branching/label logic at all.

Sure, but unlike unconditional branches, which are useful to special-case, one
sort of expects never to see an unconditional conditional move.  Given
TCG_COND_ALWAYS, we can re-use generic logic and have things fall out
relatively easily.

> I had a think about this and I couldn't really come up with a particularly
> nice looking API for it, given the way that movcondi/setcondi/brcondi work.
> The best I could come up with was something like:

The s390 target has an example with DisasCompare and disas_jcc.
The i386 target has another example with CCPrepare and gen_prepare_cc.

The i386 port uses similar flags to ARM, but represents them differently.  I
suppose good ideas could be taken from either port.


> So I definitely think I'd rather postpone this for now, unless you have
> a neat idea that I've missed for making it look nice.

Let's just postpone for now.


r~

Re: [Qemu-devel] [PATCH 01/13] target-arm: A64: add support for conditional select

[Peter Maydell]

On 6 December 2013 16:44, Richard Henderson <rth@twiddle.net> wrote:
> On 12/07/2013 01:45 AM, Peter Maydell wrote:
>> On 5 December 2013 22:26, Richard Henderson <rth@twiddle.net> wrote:
>>> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>>>> +    if (cond >= 0x0e) { /* condition "always" */
>>>> +        tcg_src = read_cpu_reg(s, rn, sf);
>>>> +        tcg_gen_mov_i64(tcg_rd, tcg_src);
>>>
>>> I wonder if it's worth adding that 0x0[ef] case to the generic condition
>>> processing rather than keep replicating it everywhere.
>>
>> I think "always true" is a special case anyway because you don't
>> want to emit any kind of branching/label logic at all.
>
> Sure, but unlike unconditional branches, which are useful to special-case, one
> sort of expects never to see an unconditional conditional move.  Given
> TCG_COND_ALWAYS, we can re-use generic logic and have things fall out
> relatively easily.

I guess. It doesn't seem much worth adding extra code to arm_gen_test_cc
that we expect to become redundant if/when we do these insns "properly"
with setcond/movcond, though, so I think it's OK like this for now.

thanks
-- PMM

[Qemu-devel] [PATCH 02/13] target-arm: A64: add support for logical (shifted register)

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

Add support for the instructions described in "C3.5.10 Logical
(shifted register)".

We store the flags in the same locations as the 32 bit decoder.
This is slightly awkward when calculating 64 bit results, but seems
a better tradeoff than having to rework the whole 32 bit decoder
and also make 32 bit result calculation in A64 awkward.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: some refactoring to avoid hidden allocation of temps,
	  rework flags, use enums for shift types,
	  renaming of functions]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |  170 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 164 insertions(+), 6 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index a9c1803..8a13806 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -36,7 +36,7 @@
 
 static TCGv_i64 cpu_X[32];
 static TCGv_i64 cpu_pc;
-static TCGv_i32 pstate;
+static TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 
 static const char *regnames[] = {
     "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7",
@@ -45,6 +45,13 @@ static const char *regnames[] = {
     "x24", "x25", "x26", "x27", "x28", "x29", "lr", "sp"
 };
 
+enum a64_shift_type {
+    A64_SHIFT_TYPE_LSL = 0,
+    A64_SHIFT_TYPE_LSR = 1,
+    A64_SHIFT_TYPE_ASR = 2,
+    A64_SHIFT_TYPE_ROR = 3
+};
+
 /* initialize TCG globals.  */
 void a64_translate_init(void)
 {
@@ -59,9 +66,10 @@ void a64_translate_init(void)
                                           regnames[i]);
     }
 
-    pstate = tcg_global_mem_new_i32(TCG_AREG0,
-                                    offsetof(CPUARMState, pstate),
-                                    "pstate");
+    cpu_NF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, NF), "NF");
+    cpu_ZF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, ZF), "ZF");
+    cpu_CF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, CF), "CF");
+    cpu_VF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, VF), "VF");
 }
 
 void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
@@ -221,6 +229,33 @@ static TCGv_i64 read_cpu_reg(DisasContext *s, int reg, int sf)
     return v;
 }
 
+/* Set ZF and NF based on a 64 bit result. This is alas fiddlier
+ * than the 32 bit equivalent.
+ */
+static inline void gen_set_NZ64(TCGv_i64 result)
+{
+    TCGv_i64 flag = tcg_temp_new_i64();
+
+    tcg_gen_setcondi_i64(TCG_COND_NE, flag, result, 0);
+    tcg_gen_trunc_i64_i32(cpu_ZF, flag);
+    tcg_gen_shri_i64(flag, result, 32);
+    tcg_gen_trunc_i64_i32(cpu_NF, flag);
+    tcg_temp_free_i64(flag);
+}
+
+/* Set NZCV as for a logical operation: NZ as per result, CV cleared. */
+static inline void gen_logic_CC(int sf, TCGv_i64 result)
+{
+    if (sf) {
+        gen_set_NZ64(result);
+    } else {
+        tcg_gen_trunc_i64_i32(cpu_ZF, result);
+        tcg_gen_trunc_i64_i32(cpu_NF, result);
+    }
+    tcg_gen_movi_i32(cpu_CF, 0);
+    tcg_gen_movi_i32(cpu_VF, 0);
+}
+
 /*
  * the instruction disassembly implemented here matches
  * the instruction encoding classifications in chapter 3 (C3)
@@ -682,10 +717,133 @@ static void disas_data_proc_imm(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Logical (shifted register) */
+/* Shift a TCGv src by TCGv shift_amount, put result in dst.
+ * Note that it is the caller's responsibility to ensure that the
+ * shift amount is in range (ie 0..31 or 0..63) and provide the ARM
+ * mandated semantics for out of range shifts.
+ */
+static void shift_reg(TCGv_i64 dst, TCGv_i64 src, int sf,
+                      enum a64_shift_type shift_type, TCGv_i64 shift_amount)
+{
+    switch (shift_type) {
+    case A64_SHIFT_TYPE_LSL:
+        tcg_gen_shl_i64(dst, src, shift_amount);
+        break;
+    case A64_SHIFT_TYPE_LSR:
+        tcg_gen_shr_i64(dst, src, shift_amount);
+        break;
+    case A64_SHIFT_TYPE_ASR:
+        if (!sf) {
+            tcg_gen_ext32s_i64(dst, src);
+        }
+        tcg_gen_sar_i64(dst, sf ? src : dst, shift_amount);
+        break;
+    case A64_SHIFT_TYPE_ROR:
+        if (sf) {
+            tcg_gen_rotr_i64(dst, src, shift_amount);
+        } else {
+            TCGv_i32 t0, t1;
+            t0 = tcg_temp_new_i32();
+            t1 = tcg_temp_new_i32();
+            tcg_gen_trunc_i64_i32(t0, src);
+            tcg_gen_trunc_i64_i32(t1, shift_amount);
+            tcg_gen_rotr_i32(t0, t0, t1);
+            tcg_gen_extu_i32_i64(dst, t0);
+            tcg_temp_free_i32(t0);
+            tcg_temp_free_i32(t1);
+        }
+        break;
+    default:
+        assert(FALSE); /* all shift types should be handled */
+        break;
+    }
+
+    if (!sf) { /* zero extend final result */
+        tcg_gen_ext32u_i64(dst, dst);
+    }
+}
+
+/* Shift a TCGv src by immediate, put result in dst.
+ * The shift amount must be in range (this should always be true as the
+ * relevant instructions will UNDEF on bad shift immediates).
+ */
+static void shift_reg_imm(TCGv_i64 dst, TCGv_i64 src, int sf,
+                          enum a64_shift_type shift_type, unsigned int shift_i)
+{
+    assert(shift_i < (sf ? 64 : 32));
+
+    if (shift_i == 0) {
+        tcg_gen_mov_i64(dst, src);
+    } else {
+        TCGv_i64 shift_const;
+
+        shift_const = tcg_const_i64(shift_i);
+        shift_reg(dst, src, sf, shift_type, shift_const);
+        tcg_temp_free_i64(shift_const);
+    }
+}
+
+/* C3.5.10 Logical (shifted register)
+ *   31  30 29 28       24 23   22 21  20  16 15    10 9    5 4    0
+ * +----+-----+-----------+-------+---+------+--------+------+------+
+ * | sf | opc | 0 1 0 1 0 | shift | N |  Rm  |  imm6  |  Rn  |  Rd  |
+ * +----+-----+-----------+-------+---+------+--------+------+------+
+ */
 static void disas_logic_reg(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    TCGv_i64 tcg_rd, tcg_rn, tcg_rm;
+    unsigned int sf, opc, shift_type, invert, rm, shift_amount, rn, rd;
+
+    sf = extract32(insn, 31, 1);
+    opc = extract32(insn, 29, 2);
+    shift_type = extract32(insn, 22, 2);
+    invert = extract32(insn, 21, 1);
+    rm = extract32(insn, 16, 5);
+    shift_amount = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+
+    if (!sf && (shift_amount & (1 << 5))) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    tcg_rm = read_cpu_reg(s, rm, sf);
+
+    if (shift_amount) {
+        shift_reg_imm(tcg_rm, tcg_rm, sf, shift_type, shift_amount);
+    }
+
+    if (invert) {
+        tcg_gen_not_i64(tcg_rm, tcg_rm);
+    }
+
+    tcg_rd = cpu_reg(s, rd);
+    tcg_rn = cpu_reg(s, rn);
+
+    switch (opc) {
+    case 0: /* AND, BIC */
+    case 3: /* ANDS, BICS */
+        tcg_gen_and_i64(tcg_rd, tcg_rn, tcg_rm);
+        break;
+    case 1: /* ORR, ORN */
+        tcg_gen_or_i64(tcg_rd, tcg_rn, tcg_rm);
+        break;
+    case 2: /* EOR, EON */
+        tcg_gen_xor_i64(tcg_rd, tcg_rn, tcg_rm);
+        break;
+    default:
+        assert(FALSE);
+        break;
+    }
+
+    if (!sf) {
+        tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+    }
+
+    if (opc == 3) {
+        gen_logic_CC(sf, tcg_rd);
+    }
 }
 
 /* Add/subtract (extended register) */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 02/13] target-arm: A64: add support for logical (shifted register)

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> +    if (invert) {
> +        tcg_gen_not_i64(tcg_rm, tcg_rm);
> +    }
> +
> +    tcg_rd = cpu_reg(s, rd);
> +    tcg_rn = cpu_reg(s, rn);
> +
> +    switch (opc) {
> +    case 0: /* AND, BIC */
> +    case 3: /* ANDS, BICS */
> +        tcg_gen_and_i64(tcg_rd, tcg_rn, tcg_rm);
> +        break;
> +    case 1: /* ORR, ORN */
> +        tcg_gen_or_i64(tcg_rd, tcg_rn, tcg_rm);
> +        break;
> +    case 2: /* EOR, EON */
> +        tcg_gen_xor_i64(tcg_rd, tcg_rn, tcg_rm);
> +        break;
> +    default:
> +        assert(FALSE);
> +        break;
> +    }

While correct, surely better to work with tcg and select on opc:invert to
generate andc/orc/eqv?

Also, isn't MOV (register) canonical for ORR (rn=31 && shift_amount=0), and MVN
(register) canonical for ORN (rn=31 && shift_amount=0), and both therefore also
worth a special case?


r~

Re: [Qemu-devel] [PATCH 02/13] target-arm: A64: add support for logical (shifted register)

[Alex Bennée]

rth@twiddle.net writes:

> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>> +    if (invert) {
>> +        tcg_gen_not_i64(tcg_rm, tcg_rm);
>> +    }
>> +
>> +    tcg_rd = cpu_reg(s, rd);
>> +    tcg_rn = cpu_reg(s, rn);
>> +
>> +    switch (opc) {
>> +    case 0: /* AND, BIC */
>> +    case 3: /* ANDS, BICS */
>> +        tcg_gen_and_i64(tcg_rd, tcg_rn, tcg_rm);
>> +        break;
>> +    case 1: /* ORR, ORN */
>> +        tcg_gen_or_i64(tcg_rd, tcg_rn, tcg_rm);
>> +        break;
>> +    case 2: /* EOR, EON */
>> +        tcg_gen_xor_i64(tcg_rd, tcg_rn, tcg_rm);
>> +        break;
>> +    default:
>> +        assert(FALSE);
>> +        break;
>> +    }
>
> While correct, surely better to work with tcg and select on opc:invert to
> generate andc/orc/eqv?

Shouldn't the TCG optimiser/back-end just be smart enough to figure it
out? It seems clearer to express the tcg ops in terms of the front-end's
meaning?

> Also, isn't MOV (register) canonical for ORR (rn=31 && shift_amount=0), and MVN
> (register) canonical for ORN (rn=31 && shift_amount=0), and both therefore also
> worth a special case?

I suspect I'm being overly cheeky to expect the optimiser to detect and
optimise for that case as the ZR is a const ;-)


Cheers,

--
Alex Bennée
QEMU/KVM Hacker for Linaro

Re: [Qemu-devel] [PATCH 02/13] target-arm: A64: add support for logical (shifted register)

[Richard Henderson]

On 12/06/2013 10:36 PM, Alex Bennée wrote:
>> While correct, surely better to work with tcg and select on opc:invert to
>> generate andc/orc/eqv?
> 
> Shouldn't the TCG optimiser/back-end just be smart enough to figure it
> out? It seems clearer to express the tcg ops in terms of the front-end's
> meaning?

No, the TCG optimizer is really quite stupid.  It only does constant folding
and dead code elimination.  No peepholing or combination sorts of opts.

>> Also, isn't MOV (register) canonical for ORR (rn=31 && shift_amount=0), and MVN
>> (register) canonical for ORN (rn=31 && shift_amount=0), and both therefore also
>> worth a special case?
> 
> I suspect I'm being overly cheeky to expect the optimiser to detect and
> optimise for that case as the ZR is a const ;-)

It would.  But since register-register move is a rather common operation, it
will pay off to not require the optimizer to clean up that special case.

Thus I only recommend special casing the official aliases, not any operation
that could mathematically be considered an identity.


r~

[Qemu-devel] [PATCH 03/13] target-arm: A64: add support for ADR and ADRP

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

Add support for the instructions described in
"C3.4.6 PC-rel. addressing" (ADR and ADRP).

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder structure]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
---
 target-arm/translate-a64.c |   25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 8a13806..bff7071 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -653,10 +653,31 @@ static void disas_ldst(DisasContext *s, uint32_t insn)
     }
 }
 
-/* PC-rel. addressing */
+/* C3.4.6 PC-rel. addressing
+ *   31  30   29 28       24 23                5 4    0
+ * +----+-------+-----------+-------------------+------+
+ * | op | immlo | 1 0 0 0 0 |       immhi       |  Rd  |
+ * +----+-------+-----------+-------------------+------+
+ */
 static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int page, rd;
+    uint64_t base;
+    int64_t offset;
+
+    page = extract32(insn, 31, 1);
+    /* SignExtend(immhi:immlo) -> offset */
+    offset = ((int64_t)sextract32(insn, 5, 19) << 2) | extract32(insn, 29, 2);
+    rd = extract32(insn, 0, 5);
+    base = s->pc - 4;
+
+    if (page) {
+        /* ADRP (page based) */
+        base &= ~0xfff;
+        offset <<= 12;
+    }
+
+    tcg_gen_movi_i64(cpu_reg(s, rd), base + offset);
 }
 
 /* Add/subtract (immediate) */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 03/13] target-arm: A64: add support for ADR and ADRP

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> Add support for the instructions described in
> "C3.4.6 PC-rel. addressing" (ADR and ADRP).
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> [claudio: adapted to new decoder structure]
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> ---
>  target-arm/translate-a64.c |   25 +++++++++++++++++++++++--
>  1 file changed, 23 inserti

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 04/13] target-arm: A64: add support for EXTR

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds emulation support for the EXTR instruction.

Signed-off-by: Alexander Graf <agraf@suse.de>

[claudio: adapted for new decoder, removed a few temporaries,
          fixed the 32bit bug, added checks for more
          unallocated cases]

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   48 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index bff7071..2ab17af 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -704,10 +704,54 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Extract */
+/* C3.4.3 Extract
+ *   31  30  29 28         23 22   21  20  16 15    10 9    5 4    0
+ * +----+------+-------------+---+----+------+--------+------+------+
+ * | sf | op21 | 1 0 0 1 1 1 | N | o0 |  Rm  |  imms  |  Rn  |  Rd  |
+ * +----+------+-------------+---+----+------+--------+------+------+
+ */
 static void disas_extract(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, n, rm, imm, rn, rd, bitsize, op21, op0;
+
+    sf = extract32(insn, 31, 1);
+    n = extract32(insn, 22, 1);
+    rm = extract32(insn, 16, 5);
+    imm = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+    op21 = extract32(insn, 29, 2);
+    op0 = extract32(insn, 21, 1);
+    bitsize = sf ? 64 : 32;
+
+    if (sf != n || op21 || op0 || imm >= bitsize) {
+        unallocated_encoding(s);
+    } else {
+        TCGv_i64 tcg_rd, tcg_rm, tcg_rn;
+
+        tcg_rd = cpu_reg(s, rd);
+
+        if (imm) {
+            tcg_rm = read_cpu_reg(s, rm, sf);
+            tcg_rn = read_cpu_reg(s, rn, sf);
+            tcg_gen_shri_i64(tcg_rm, tcg_rm, imm);
+            tcg_gen_shli_i64(tcg_rn, tcg_rn, bitsize - imm);
+            tcg_gen_or_i64(tcg_rd, tcg_rm, tcg_rn);
+            if (!sf) {
+                tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+            }
+        } else {
+            /* tcg shl_i32/shl_i64 is undefined for 32/64 bit shifts,
+             * so an extract from bit 0 is a special case.
+             */
+            if (sf) {
+                tcg_gen_mov_i64(tcg_rd, cpu_reg(s, rm));
+            } else {
+                tcg_gen_ext32u_i64(tcg_rd, cpu_reg(s, rm));
+            }
+        }
+
+    }
 }
 
 /* C3.4 Data processing - immediate */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 04/13] target-arm: A64: add support for EXTR

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> This patch adds emulation support for the EXTR instruction.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> [claudio: adapted for new decoder, removed a few temporaries,
>           fixed the 32bit bug, added checks for more
>           unallocated cases]
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |   48 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>

> +            tcg_rm = read_cpu_reg(s, rm, sf);
> +            tcg_rn = read_cpu_reg(s, rn, sf);
> +            tcg_gen_shri_i64(tcg_rm, tcg_rm, imm);
> +            tcg_gen_shli_i64(tcg_rn, tcg_rn, bitsize - imm);
> +            tcg_gen_or_i64(tcg_rd, tcg_rm, tcg_rn);
> +            if (!sf) {
> +                tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
> +            }

OPTME: If Rm==Rn, this is a rotate.


r~

[Qemu-devel] [PATCH 05/13] target-arm: A64: add support for 2-src data processing and DIV

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds support for decoding 2-src data processing insns,
and the first users, UDIV and SDIV.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder adding the 2-src decoding level,
          always zero-extend result in 32bit mode]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper-a64.c    |   21 +++++++++++++
 target-arm/helper-a64.h    |    2 ++
 target-arm/translate-a64.c |   72 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index adb8428..abb98c0 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -23,3 +23,24 @@
 #include "qemu/host-utils.h"
 #include "sysemu/sysemu.h"
 #include "qemu/bitops.h"
+
+/* C2.4.7 Multiply and divide */
+/* special cases for 0 and LLONG_MIN are mandated by the standard */
+uint64_t HELPER(udiv64)(uint64_t num, uint64_t den)
+{
+    if (den == 0) {
+        return 0;
+    }
+    return num / den;
+}
+
+int64_t HELPER(sdiv64)(int64_t num, int64_t den)
+{
+    if (den == 0) {
+        return 0;
+    }
+    if (num == LLONG_MIN && den == -1) {
+        return LLONG_MIN;
+    }
+    return num / den;
+}
diff --git a/target-arm/helper-a64.h b/target-arm/helper-a64.h
index dd28306..e0d6506 100644
--- a/target-arm/helper-a64.h
+++ b/target-arm/helper-a64.h
@@ -16,3 +16,5 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+DEF_HELPER_FLAGS_2(udiv64, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(sdiv64, TCG_CALL_NO_RWG_SE, s64, s64, s64)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 2ab17af..43725a9 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1011,10 +1011,78 @@ static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Data-processing (2 source) */
+static void handle_div(DisasContext *s, bool is_signed, unsigned int sf,
+                       unsigned int rm, unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_n, tcg_m, tcg_rd;
+    tcg_rd = cpu_reg(s, rd);
+
+    if (!sf && is_signed) {
+        tcg_n = new_tmp_a64(s);
+        tcg_m = new_tmp_a64(s);
+        tcg_gen_ext32s_i64(tcg_n, cpu_reg(s, rn));
+        tcg_gen_ext32s_i64(tcg_m, cpu_reg(s, rm));
+    } else {
+        tcg_n = read_cpu_reg(s, rn, sf);
+        tcg_m = read_cpu_reg(s, rm, sf);
+    }
+
+    if (is_signed) {
+        gen_helper_sdiv64(tcg_rd, tcg_n, tcg_m);
+    } else {
+        gen_helper_udiv64(tcg_rd, tcg_n, tcg_m);
+    }
+
+    if (!sf) { /* zero extend final result */
+        tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+    }
+}
+
+/* C3.5.8 Data-processing (2 source)
+ *   31   30  29 28             21 20  16 15    10 9    5 4    0
+ * +----+---+---+-----------------+------+--------+------+------+
+ * | sf | 0 | S | 1 1 0 1 0 1 1 0 |  Rm  | opcode |  Rn  |  Rd  |
+ * +----+---+---+-----------------+------+--------+------+------+
+ */
 static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, rm, opcode, rn, rd;
+    sf = extract32(insn, 31, 1);
+    rm = extract32(insn, 16, 5);
+    opcode = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+
+    if (extract32(insn, 29, 1)) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    switch (opcode) {
+    case 2: /* UDIV */
+        handle_div(s, FALSE, sf, rm, rn, rd);
+        break;
+    case 3: /* SDIV */
+        handle_div(s, TRUE, sf, rm, rn, rd);
+        break;
+    case 8: /* LSLV */
+    case 9: /* LSRV */
+    case 10: /* ASRV */
+    case 11: /* RORV */
+    case 16:
+    case 17:
+    case 18:
+    case 19:
+    case 20:
+    case 21:
+    case 22:
+    case 23: /* CRC32 */
+        unsupported_encoding(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
 }
 
 /* C3.5 Data processing - register */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 05/13] target-arm: A64: add support for 2-src data processing and DIV

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> +    switch (opcode) {
> +    case 2: /* UDIV */
> +        handle_div(s, FALSE, sf, rm, rn, rd);
> +        break;
> +    case 3: /* SDIV */
> +        handle_div(s, TRUE, sf, rm, rn, rd);
> +        break;

What are these all-caps TRUE/FALSE?  stdbool.h uses lower-case.

Otherwise,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 05/13] target-arm: A64: add support for 2-src data processing and DIV

[Peter Maydell]

On 5 December 2013 22:51, Richard Henderson <rth@twiddle.net> wrote:
> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>> +    switch (opcode) {
>> +    case 2: /* UDIV */
>> +        handle_div(s, FALSE, sf, rm, rn, rd);
>> +        break;
>> +    case 3: /* SDIV */
>> +        handle_div(s, TRUE, sf, rm, rn, rd);
>> +        break;
>
> What are these all-caps TRUE/FALSE?  stdbool.h uses lower-case.

Good question, I wonder what system header is managing to define
those for us? (there are some other bits of the source tree which
use them too I see).

> Otherwise,
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>

By the way, for these "otherwise reviewed-by" patches, would
you prefer me to make the obvious trivial fix and include your
R-b tag on the fixed version in the next respin, or to make the
fix and leave the tag off so you can recheck it?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 05/13] target-arm: A64: add support for 2-src data processing and DIV

[Richard Henderson]

On 12/06/2013 12:09 PM, Peter Maydell wrote:
> On 5 December 2013 22:51, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>>> +    switch (opcode) {
>>> +    case 2: /* UDIV */
>>> +        handle_div(s, FALSE, sf, rm, rn, rd);
>>> +        break;
>>> +    case 3: /* SDIV */
>>> +        handle_div(s, TRUE, sf, rm, rn, rd);
>>> +        break;
>>
>> What are these all-caps TRUE/FALSE?  stdbool.h uses lower-case.
> 
> Good question, I wonder what system header is managing to define
> those for us? (there are some other bits of the source tree which
> use them too I see).
> 
>> Otherwise,
>>
>> Reviewed-by: Richard Henderson <rth@twiddle.net>
> 
> By the way, for these "otherwise reviewed-by" patches, would
> you prefer me to make the obvious trivial fix and include your
> R-b tag on the fixed version in the next respin, or to make the
> fix and leave the tag off so you can recheck it?

The former -- make the trivial fix and include the R-b.


r~

Re: [Qemu-devel] [PATCH 05/13] target-arm: A64: add support for 2-src data processing and DIV

[C Fontana]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On Friday, December 6, 2013, Peter Maydell wrote:

> On 5 December 2013 22:51, Richard Henderson <rth@twiddle.net<javascript:;>>
> wrote:
> > On 12/06/2013 10:51 AM, Peter Maydell wrote:
> >> +    switch (opcode) {
> >> +    case 2: /* UDIV */
> >> +        handle_div(s, FALSE, sf, rm, rn, rd);
> >> +        break;
> >> +    case 3: /* SDIV */
> >> +        handle_div(s, TRUE, sf, rm, rn, rd);
> >> +        break;
> >
> > What are these all-caps TRUE/FALSE?  stdbool.h uses lower-case.
>
> Good question, I wonder what system header is managing to define
> those for us? (there are some other bits of the source tree which
> use them too I see).
>
> Glib, this habit comes from GTK-related programming.

Ciao, C.


> > Otherwise,
> >
> > Reviewed-by: Richard Henderson <rth@twiddle.net <javascript:;>>
>
> By the way, for these "otherwise reviewed-by" patches, would
> you prefer me to make the obvious trivial fix and include your
> R-b tag on the fixed version in the next respin, or to make the
> fix and leave the tag off so you can recheck it?
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 1678 bytes --]

Re: [Qemu-devel] [PATCH 05/13] target-arm: A64: add support for 2-src data processing and DIV

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

On 12/05/2013 04:09 PM, Peter Maydell wrote:
> On 5 December 2013 22:51, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/06/2013 10:51 AM, Peter Maydell wrote:
>>> +    switch (opcode) {
>>> +    case 2: /* UDIV */
>>> +        handle_div(s, FALSE, sf, rm, rn, rd);
>>> +        break;
>>> +    case 3: /* SDIV */
>>> +        handle_div(s, TRUE, sf, rm, rn, rd);
>>> +        break;
>>
>> What are these all-caps TRUE/FALSE?  stdbool.h uses lower-case.
> 
> Good question, I wonder what system header is managing to define
> those for us? (there are some other bits of the source tree which
> use them too I see).

I blame glib:

https://developer.gnome.org/glib/2.32/glib-Standard-Macros.html#TRUE:CAPS

There are cases in the code where we must use TRUE because we want to
ensure we are using the gboolean type (which is not necessarily the same
size as bool), but I also concur that the use of <stdbool.h> true is
much nicer than TRUE in any code unrelated to glib.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

[Qemu-devel] [PATCH 06/13] target-arm: A64: add support for 2-src shift reg insns

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This adds 2-src variable shift register instructions:
C5.6.115 LSLV, C5.6.118 LSRV, C5.6.17 ASRV, C5.6.154 RORV

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder, use enums for shift types]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 43725a9..c2d0308 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1038,6 +1038,20 @@ static void handle_div(DisasContext *s, bool is_signed, unsigned int sf,
     }
 }
 
+/* C5.6.115 LSLV, C5.6.118 LSRV, C5.6.17 ASRV, C5.6.154 RORV */
+static void handle_shift_reg(DisasContext *s,
+                             enum a64_shift_type shift_type, unsigned int sf,
+                             unsigned int rm, unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_shift = tcg_temp_new_i64();
+    TCGv_i64 tcg_rd = cpu_reg(s, rd);
+    TCGv_i64 tcg_rn = read_cpu_reg(s, rn, sf);
+
+    tcg_gen_andi_i64(tcg_shift, cpu_reg(s, rm), sf ? 63 : 31);
+    shift_reg(tcg_rd, tcg_rn, sf, shift_type, tcg_shift);
+    tcg_temp_free_i64(tcg_shift);
+}
+
 /* C3.5.8 Data-processing (2 source)
  *   31   30  29 28             21 20  16 15    10 9    5 4    0
  * +----+---+---+-----------------+------+--------+------+------+
@@ -1066,9 +1080,17 @@ static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
         handle_div(s, TRUE, sf, rm, rn, rd);
         break;
     case 8: /* LSLV */
+        handle_shift_reg(s, A64_SHIFT_TYPE_LSL, sf, rm, rn, rd);
+        break;
     case 9: /* LSRV */
+        handle_shift_reg(s, A64_SHIFT_TYPE_LSR, sf, rm, rn, rd);
+        break;
     case 10: /* ASRV */
+        handle_shift_reg(s, A64_SHIFT_TYPE_ASR, sf, rm, rn, rd);
+        break;
     case 11: /* RORV */
+        handle_shift_reg(s, A64_SHIFT_TYPE_ROR, sf, rm, rn, rd);
+        break;
     case 16:
     case 17:
     case 18:
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 06/13] target-arm: A64: add support for 2-src shift reg insns

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> This adds 2-src variable shift register instructions:
> C5.6.115 LSLV, C5.6.118 LSRV, C5.6.17 ASRV, C5.6.154 RORV
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> [claudio: adapted to new decoder, use enums for shift types]
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 07/13] target-arm: A64: add support for 1-src data processing and CLZ

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

This patch adds support for decoding 1-src data processing insns,
and the first user, C5.6.40 CLZ (count leading zeroes).

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper-a64.c    |    5 +++++
 target-arm/helper-a64.h    |    1 +
 target-arm/translate-a64.c |   52 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index abb98c0..e4c5346 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -44,3 +44,8 @@ int64_t HELPER(sdiv64)(int64_t num, int64_t den)
     }
     return num / den;
 }
+
+uint64_t HELPER(clz64)(uint64_t x)
+{
+    return clz64(x);
+}
diff --git a/target-arm/helper-a64.h b/target-arm/helper-a64.h
index e0d6506..b10b6c3 100644
--- a/target-arm/helper-a64.h
+++ b/target-arm/helper-a64.h
@@ -18,3 +18,4 @@
  */
 DEF_HELPER_FLAGS_2(udiv64, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sdiv64, TCG_CALL_NO_RWG_SE, s64, s64, s64)
+DEF_HELPER_FLAGS_1(clz64, TCG_CALL_NO_RWG_SE, i64, i64)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index c2d0308..3ee2674 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1005,10 +1005,58 @@ static void disas_cond_select(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Data-processing (1 source) */
+static void handle_clz(DisasContext *s, unsigned int sf,
+                       unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_rd, tcg_rn;
+    tcg_rd = cpu_reg(s, rd);
+    tcg_rn = cpu_reg(s, rn);
+
+    if (sf) {
+        gen_helper_clz64(tcg_rd, tcg_rn);
+    } else {
+        TCGv_i32 tcg_tmp32 = tcg_temp_new_i32();
+        tcg_gen_trunc_i64_i32(tcg_tmp32, tcg_rn);
+        gen_helper_clz(tcg_tmp32, tcg_tmp32);
+        tcg_gen_extu_i32_i64(tcg_rd, tcg_tmp32);
+        tcg_temp_free_i32(tcg_tmp32);
+    }
+}
+
+/* C3.5.7 Data-processing (1 source)
+ *   31  30  29  28             21 20     16 15    10 9    5 4    0
+ * +----+---+---+-----------------+---------+--------+------+------+
+ * | sf | 1 | S | 1 1 0 1 0 1 1 0 | opcode2 | opcode |  Rn  |  Rd  |
+ * +----+---+---+-----------------+---------+--------+------+------+
+ */
 static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, opcode, rn, rd;
+
+    if (extract32(insn, 29, 1) || extract32(insn, 16, 5)) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    sf = extract32(insn, 31, 1);
+    opcode = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+
+    switch (opcode) {
+    case 0: /* RBIT */
+    case 1: /* REV16 */
+    case 2: /* REV32 */
+    case 3: /* REV64 */
+        unsupported_encoding(s, insn);
+        break;
+    case 4: /* CLZ */
+        handle_clz(s, sf, rn, rd);
+        break;
+    case 5: /* CLS */
+        unsupported_encoding(s, insn);
+        break;
+    }
 }
 
 static void handle_div(DisasContext *s, bool is_signed, unsigned int sf,
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 07/13] target-arm: A64: add support for 1-src data processing and CLZ

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> This patch adds support for decoding 1-src data processing insns,
> and the first user, C5.6.40 CLZ (count leading zeroes).
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper-a64.c    |    5 +++++
>  target-arm/helper-a64.h    |    1 +
>  target-arm/translate-a64.c |   52 ++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 56 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 08/13] target-arm: A64: add support for 1-src RBIT insn

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This adds support for the C5.6.147 RBIT instruction.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder, use bswap64,
          make RBIT part standalone from the rest of the patch,
	  splitting REV into a separate patch]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper-a64.c    |   18 ++++++++++++++++++
 target-arm/helper-a64.h    |    1 +
 target-arm/translate-a64.c |   20 ++++++++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index e4c5346..cccaac6 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -49,3 +49,21 @@ uint64_t HELPER(clz64)(uint64_t x)
 {
     return clz64(x);
 }
+
+uint64_t HELPER(rbit64)(uint64_t x)
+{
+    /* assign the correct byte position */
+    x = bswap64(x);
+
+    /* assign the correct nibble position */
+    x = ((x & 0xf0f0f0f0f0f0f0f0ULL) >> 4)
+        | ((x & 0x0f0f0f0f0f0f0f0fULL) << 4);
+
+    /* assign the correct bit position */
+    x = ((x & 0x8888888888888888ULL) >> 3)
+        | ((x & 0x4444444444444444ULL) >> 1)
+        | ((x & 0x2222222222222222ULL) << 1)
+        | ((x & 0x1111111111111111ULL) << 3);
+
+    return x;
+}
diff --git a/target-arm/helper-a64.h b/target-arm/helper-a64.h
index b10b6c3..9959139 100644
--- a/target-arm/helper-a64.h
+++ b/target-arm/helper-a64.h
@@ -19,3 +19,4 @@
 DEF_HELPER_FLAGS_2(udiv64, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sdiv64, TCG_CALL_NO_RWG_SE, s64, s64, s64)
 DEF_HELPER_FLAGS_1(clz64, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 3ee2674..de3e14a 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1023,6 +1023,24 @@ static void handle_clz(DisasContext *s, unsigned int sf,
     }
 }
 
+static void handle_rbit(DisasContext *s, unsigned int sf,
+                        unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_rd, tcg_rn;
+    tcg_rd = cpu_reg(s, rd);
+    tcg_rn = cpu_reg(s, rn);
+
+    if (sf) {
+        gen_helper_rbit64(tcg_rd, tcg_rn);
+    } else {
+        TCGv_i32 tcg_tmp32 = tcg_temp_new_i32();
+        tcg_gen_trunc_i64_i32(tcg_tmp32, tcg_rn);
+        gen_helper_rbit(tcg_tmp32, tcg_tmp32);
+        tcg_gen_extu_i32_i64(tcg_rd, tcg_tmp32);
+        tcg_temp_free_i32(tcg_tmp32);
+    }
+}
+
 /* C3.5.7 Data-processing (1 source)
  *   31  30  29  28             21 20     16 15    10 9    5 4    0
  * +----+---+---+-----------------+---------+--------+------+------+
@@ -1045,6 +1063,8 @@ static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
 
     switch (opcode) {
     case 0: /* RBIT */
+        handle_rbit(s, sf, rn, rd);
+        break;
     case 1: /* REV16 */
     case 2: /* REV32 */
     case 3: /* REV64 */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 08/13] target-arm: A64: add support for 1-src RBIT insn

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> This adds support for the C5.6.147 RBIT instruction.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> [claudio: adapted to new decoder, use bswap64,
>           make RBIT part standalone from the rest of the patch,
> 	  splitting REV into a separate patch]
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper-a64.c    |   18 ++++++++++++++++++
>  target-arm/helper-a64.h    |    1 +
>  target-arm/translate-a64.c |   20 ++++++++++++++++++++
>  3 files changed, 39 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 09/13] target-arm: A64: add support for 1-src REV insns

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

This adds support for C5.6.149 REV, C5.6.151 REV32, C5.6.150 REV16.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   73 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index de3e14a..2720fd6 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1041,6 +1041,73 @@ static void handle_rbit(DisasContext *s, unsigned int sf,
     }
 }
 
+/* C5.6.149 REV with sf==1, opcode==3 ("REV64") */
+static void handle_rev64(DisasContext *s, unsigned int sf,
+                         unsigned int rn, unsigned int rd)
+{
+    if (!sf) {
+        unallocated_encoding(s);
+        return;
+    }
+    tcg_gen_bswap64_i64(cpu_reg(s, rd), cpu_reg(s, rn));
+}
+
+/* C5.6.149 REV with sf==0, opcode==2
+ * C5.6.151 REV32 (sf==1, opcode==2)
+ */
+static void handle_rev32(DisasContext *s, unsigned int sf,
+                         unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_rd = cpu_reg(s, rd);
+
+    if (sf) {
+        TCGv_i64 tcg_tmp = tcg_temp_new_i64();
+        TCGv_i64 tcg_rn = read_cpu_reg(s, rn, sf);
+
+        /* bswap32_i64 requires zero high word */
+        tcg_gen_ext32u_i64(tcg_tmp, tcg_rn);
+        tcg_gen_bswap32_i64(tcg_rd, tcg_tmp);
+        tcg_gen_shri_i64(tcg_tmp, tcg_rn, 32);
+        tcg_gen_bswap32_i64(tcg_tmp, tcg_tmp);
+        tcg_gen_concat32_i64(tcg_rd, tcg_rd, tcg_tmp);
+
+        tcg_temp_free_i64(tcg_tmp);
+    } else {
+        tcg_gen_ext32u_i64(tcg_rd, cpu_reg(s, rn));
+        tcg_gen_bswap32_i64(tcg_rd, tcg_rd);
+    }
+}
+
+/* C5.6.150 REV16 (opcode==1) */
+static void handle_rev16(DisasContext *s, unsigned int sf,
+                         unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_rd = cpu_reg(s, rd);
+    TCGv_i64 tcg_tmp = tcg_temp_new_i64();
+    TCGv_i64 tcg_rn = read_cpu_reg(s, rn, sf);
+
+    tcg_gen_andi_i64(tcg_tmp, tcg_rn, 0xffff);
+    tcg_gen_bswap16_i64(tcg_rd, tcg_tmp);
+
+    tcg_gen_shri_i64(tcg_tmp, tcg_rn, 16);
+    tcg_gen_andi_i64(tcg_tmp, tcg_tmp, 0xffff);
+    tcg_gen_bswap16_i64(tcg_tmp, tcg_tmp);
+    tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, 16, 16);
+
+    if (sf) {
+        tcg_gen_shri_i64(tcg_tmp, tcg_rn, 32);
+        tcg_gen_andi_i64(tcg_tmp, tcg_tmp, 0xffff);
+        tcg_gen_bswap16_i64(tcg_tmp, tcg_tmp);
+        tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, 32, 16);
+
+        tcg_gen_shri_i64(tcg_tmp, tcg_rn, 48);
+        tcg_gen_bswap16_i64(tcg_tmp, tcg_tmp);
+        tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, 48, 16);
+    }
+
+    tcg_temp_free_i64(tcg_tmp);
+}
+
 /* C3.5.7 Data-processing (1 source)
  *   31  30  29  28             21 20     16 15    10 9    5 4    0
  * +----+---+---+-----------------+---------+--------+------+------+
@@ -1066,9 +1133,13 @@ static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
         handle_rbit(s, sf, rn, rd);
         break;
     case 1: /* REV16 */
+        handle_rev16(s, sf, rn, rd);
+        break;
     case 2: /* REV32 */
+        handle_rev32(s, sf, rn, rd);
+        break;
     case 3: /* REV64 */
-        unsupported_encoding(s, insn);
+        handle_rev64(s, sf, rn, rd);
         break;
     case 4: /* CLZ */
         handle_clz(s, sf, rn, rd);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 09/13] target-arm: A64: add support for 1-src REV insns

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> This adds support for C5.6.149 REV, C5.6.151 REV32, C5.6.150 REV16.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |   73 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 10/13] target-arm: A64: add support for bitfield insns

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

This patch implements the C3.4.2 Bitfield instructions:
SBFM, BFM, UBFM.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   54 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 2720fd6..ccf914e 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -698,10 +698,60 @@ static void disas_movw_imm(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Bitfield */
+/* C3.4.2 Bitfield
+ *   31  30 29 28         23 22  21  16 15  10 9    5 4    0
+ * +----+-----+-------------+---+------+------+------+------+
+ * | sf | opc | 1 0 0 1 1 0 | N | immr | imms |  Rn  |  Rd  |
+ * +----+-----+-------------+---+------+------+------+------+
+ */
 static void disas_bitfield(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, n, opc, ri, si, rn, rd, bitsize, pos, len;
+    TCGv_i64 tcg_rd, tcg_tmp;
+
+    sf = extract32(insn, 31, 1);
+    opc = extract32(insn, 29, 2);
+    n = extract32(insn, 22, 1);
+    ri = extract32(insn, 16, 6);
+    si = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+    bitsize = sf ? 64 : 32;
+
+    if (sf != n || ri >= bitsize || si >= bitsize || opc > 2) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    tcg_rd = cpu_reg(s, rd);
+    tcg_tmp = read_cpu_reg(s, rn, sf);
+
+    if (opc != 1) { /* SBFM or UBFM */
+        tcg_gen_movi_i64(tcg_rd, 0);
+    }
+
+    /* do the bit move operation */
+    if (si >= ri) {
+        /* Wd<s-r:0> = Wn<s:r> */
+        tcg_gen_shri_i64(tcg_tmp, tcg_tmp, ri);
+        pos = 0;
+        len = (si - ri) + 1;
+    } else {
+        /* Wd<32+s-r,32-r> = Wn<s:0> */
+        pos = bitsize - ri;
+        len = si + 1;
+    }
+
+    tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, pos, len);
+
+    if (opc == 0) { /* SBFM - sign extend the destination field */
+        tcg_gen_shli_i64(tcg_rd, tcg_rd, 64 - (pos + len));
+        tcg_gen_sari_i64(tcg_rd, tcg_rd, 64 - (pos + len));
+    }
+
+    if (!sf) { /* zero extend final result */
+        tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+    }
 }
 
 /* C3.4.3 Extract
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 10/13] target-arm: A64: add support for bitfield insns

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> This patch implements the C3.4.2 Bitfield instructions:
> SBFM, BFM, UBFM.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |   54 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>

> +    if (opc != 1) { /* SBFM or UBFM */
> +        tcg_gen_movi_i64(tcg_rd, 0);
> +    }
> +
> +    /* do the bit move operation */
> +    if (si >= ri) {
> +        /* Wd<s-r:0> = Wn<s:r> */
> +        tcg_gen_shri_i64(tcg_tmp, tcg_tmp, ri);
> +        pos = 0;
> +        len = (si - ri) + 1;
> +    } else {
> +        /* Wd<32+s-r,32-r> = Wn<s:0> */
> +        pos = bitsize - ri;
> +        len = si + 1;
> +    }
> +
> +    tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, pos, len);
> +
> +    if (opc == 0) { /* SBFM - sign extend the destination field */
> +        tcg_gen_shli_i64(tcg_rd, tcg_rd, 64 - (pos + len));
> +        tcg_gen_sari_i64(tcg_rd, tcg_rd, 64 - (pos + len));
> +    }

OPTME: It's probably worth recognizing the common cases of ext{8,16,32}{u,s}.


r~

[Qemu-devel] [PATCH 11/13] host-utils: add clrsb32/64 - count leading redundant sign bits

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

this patch introduces wrappers for the clrsb builtins,
which count the leading redundant sign bits.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/host-utils.h |   32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 0f688c1..de85d28 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -228,6 +228,38 @@ static inline int cto64(uint64_t val)
 }
 
 /**
+ * clrsb32 - count leading redundant sign bits in a 32-bit value.
+ * @val: The value to search
+ *
+ * Returns the number of bits following the sign bit that are equal to it.
+ * No special cases; output range is [0-31].
+ */
+static inline int clrsb32(uint32_t val)
+{
+#if QEMU_GNUC_PREREQ(4, 7)
+    return __builtin_clrsb(val);
+#else
+    return clz32(val ^ ((int32_t)val >> 1)) - 1;
+#endif
+}
+
+/**
+ * clrsb64 - count leading redundant sign bits in a 64-bit value.
+ * @val: The value to search
+ *
+ * Returns the number of bits following the sign bit that are equal to it.
+ * No special cases; output range is [0-63].
+ */
+static inline int clrsb64(uint64_t val)
+{
+#if QEMU_GNUC_PREREQ(4, 7)
+    return __builtin_clrsbll(val);
+#else
+    return clz64(val ^ ((int64_t)val >> 1)) - 1;
+#endif
+}
+
+/**
  * ctpop8 - count the population of one bits in an 8-bit value.
  * @val: The value to search
  */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 11/13] host-utils: add clrsb32/64 - count leading redundant sign bits

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> this patch introduces wrappers for the clrsb builtins,
> which count the leading redundant sign bits.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/host-utils.h |   32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~!

[Qemu-devel] [PATCH 12/13] target-arm: A64: add support for 1-src CLS insn

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

this patch adds support for the CLS instruction.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper-a64.c    |   10 ++++++++++
 target-arm/helper-a64.h    |    2 ++
 target-arm/translate-a64.c |   20 +++++++++++++++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index cccaac6..d3f7067 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -50,6 +50,16 @@ uint64_t HELPER(clz64)(uint64_t x)
     return clz64(x);
 }
 
+uint64_t HELPER(cls64)(uint64_t x)
+{
+    return clrsb64(x);
+}
+
+uint32_t HELPER(cls32)(uint32_t x)
+{
+    return clrsb32(x);
+}
+
 uint64_t HELPER(rbit64)(uint64_t x)
 {
     /* assign the correct byte position */
diff --git a/target-arm/helper-a64.h b/target-arm/helper-a64.h
index 9959139..a163a94 100644
--- a/target-arm/helper-a64.h
+++ b/target-arm/helper-a64.h
@@ -19,4 +19,6 @@
 DEF_HELPER_FLAGS_2(udiv64, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sdiv64, TCG_CALL_NO_RWG_SE, s64, s64, s64)
 DEF_HELPER_FLAGS_1(clz64, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_FLAGS_1(cls64, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_FLAGS_1(cls32, TCG_CALL_NO_RWG_SE, i32, i32)
 DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index ccf914e..a7ca27c 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1073,6 +1073,24 @@ static void handle_clz(DisasContext *s, unsigned int sf,
     }
 }
 
+static void handle_cls(DisasContext *s, unsigned int sf,
+                       unsigned int rn, unsigned int rd)
+{
+    TCGv_i64 tcg_rd, tcg_rn;
+    tcg_rd = cpu_reg(s, rd);
+    tcg_rn = cpu_reg(s, rn);
+
+    if (sf) {
+        gen_helper_cls64(tcg_rd, tcg_rn);
+    } else {
+        TCGv_i32 tcg_tmp32 = tcg_temp_new_i32();
+        tcg_gen_trunc_i64_i32(tcg_tmp32, tcg_rn);
+        gen_helper_cls32(tcg_tmp32, tcg_tmp32);
+        tcg_gen_extu_i32_i64(tcg_rd, tcg_tmp32);
+        tcg_temp_free_i32(tcg_tmp32);
+    }
+}
+
 static void handle_rbit(DisasContext *s, unsigned int sf,
                         unsigned int rn, unsigned int rd)
 {
@@ -1195,7 +1213,7 @@ static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
         handle_clz(s, sf, rn, rd);
         break;
     case 5: /* CLS */
-        unsupported_encoding(s, insn);
+        handle_cls(s, sf, rn, rd);
         break;
     }
 }
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 12/13] target-arm: A64: add support for 1-src CLS insn

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> this patch adds support for the CLS instruction.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper-a64.c    |   10 ++++++++++
>  target-arm/helper-a64.h    |    2 ++
>  target-arm/translate-a64.c |   20 +++++++++++++++++++-
>  3 files changed, 31 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 13/13] target-arm: A64: add support for logical (immediate) insns

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds support for C3.4.4 Logical (immediate),
which include AND, ANDS, ORR, EOR.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder, function renaming,
          removed a TCG temp variable]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
[PMM: cleaned up some unnecessary code in logic_imm_decode_wmask
and added clarifying commentary on what it's actually doing.
Dropped an ext32u that's not needed if we've just done an AND.]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |  175 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 173 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index a7ca27c..d03cfc0 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -201,6 +201,21 @@ static TCGv_i64 new_tmp_a64_zero(DisasContext *s)
     return t;
 }
 
+/*
+ * Register access functions
+ *
+ * These functions are used for directly accessing a register in where
+ * changes to the final register value are likely to be made. If you
+ * need to use a register for temporary calculation (e.g. index type
+ * operations) use the read_* form.
+ *
+ * B1.2.1 Register mappings
+ *
+ * In instruction register encoding 31 can refer to ZR (zero register) or
+ * the SP (stack pointer) depending on context. In QEMU's case we map SP
+ * to cpu_X[31] and ZR accesses to a temporary which can be discarded.
+ * This is the point of the _sp forms.
+ */
 static TCGv_i64 cpu_reg(DisasContext *s, int reg)
 {
     if (reg == 31) {
@@ -210,6 +225,12 @@ static TCGv_i64 cpu_reg(DisasContext *s, int reg)
     }
 }
 
+/* register access for when 31 == SP */
+static TCGv_i64 cpu_reg_sp(DisasContext *s, int reg)
+{
+    return cpu_X[reg];
+}
+
 /* read a cpu register in 32bit/64bit mode. Returns a TCGv_i64
  * representing the register contents. This TCGv is an auto-freed
  * temporary so it need not be explicitly freed, and may be modified.
@@ -686,10 +707,160 @@ static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Logical (immediate) */
+/* The input should be a value in the bottom e bits (with higher
+ * bits zero); returns that value replicated into every element
+ * of size e in a 64 bit integer.
+ */
+static uint64_t bitfield_replicate(uint64_t mask, unsigned int e)
+{
+    assert(e != 0);
+    while (e < 64) {
+        mask |= mask << e;
+        e *= 2;
+    }
+    return mask;
+}
+
+/* Return a value with the bottom len bits set (where 0 < len <= 64) */
+static inline uint64_t bitmask64(unsigned int length)
+{
+    assert(length > 0 && length <= 64);
+    return ~0ULL >> (64 - length);
+}
+
+/* Simplified variant of pseudocode DecodeBitMasks() for the case where we
+ * only require the wmask. Returns false if the imms/immr/immn are a reserved
+ * value (ie should cause a guest UNDEF exception), and true if they are
+ * valid, in which case the decoded bit pattern is written to result.
+ */
+static bool logic_imm_decode_wmask(uint64_t *result, unsigned int immn,
+                                   unsigned int imms, unsigned int immr)
+{
+    uint64_t mask;
+    unsigned e, levels, s, r;
+    int len;
+
+    assert(immn < 2 && imms < 64 && immr < 64);
+
+    /* The bit patterns we create here are 64 bit patterns which
+     * are vectors of identical elements of size e = 2, 4, 8, 16, 32 or
+     * 64 bits each. Each element contains the same value: a run
+     * of between 1 and e-1 non-zero bits, rotated within the
+     * element by between 0 and e-1 bits.
+     *
+     * The element size and run length are encoded into immn (1 bit)
+     * and imms (6 bits) as follows:
+     * 64 bit elements: immn = 1, imms = <length of run - 1>
+     * 32 bit elements: immn = 0, imms = 0 : <length of run - 1>
+     * 16 bit elements: immn = 0, imms = 10 : <length of run - 1>
+     *  8 bit elements: immn = 0, imms = 110 : <length of run - 1>
+     *  4 bit elements: immn = 0, imms = 1110 : <length of run - 1>
+     *  2 bit elements: immn = 0, imms = 11110 : <length of run - 1>
+     * Notice that immn = 0, imms = 11111x is the only combination
+     * not covered by one of the above options; this is reserved.
+     * Further, <length of run - 1> all-ones is a reserved pattern.
+     *
+     * In all cases the rotation is by immr % e (and immr is 6 bits).
+     */
+
+    /* First determine the element size */
+    len = 31 - clz32((immn << 6) | (~imms & 0x3f));
+    if (len < 1) {
+        /* This is the immn == 0, imms == 0x11111x case */
+        return false;
+    }
+    e = 1 << len;
+
+    levels = e - 1;
+    s = imms & levels;
+    r = immr & levels;
+
+    if (s == levels) {
+        /* <length of run - 1> mustn't be all-ones. */
+        return false;
+    }
+
+    /* Create the value of one element: s+1 set bits rotated
+     * by r within the element (which is e bits wide)...
+     */
+    mask = bitmask64(s + 1);
+    mask = (mask >> r) | (mask << (e - r));
+    /* ...then replicate the element over the whole 64 bit value */
+    mask = bitfield_replicate(mask, e);
+    *result = mask;
+    return true;
+}
+
+/* C3.4.4 Logical (immediate)
+ *   31  30 29 28         23 22  21  16 15  10 9    5 4    0
+ * +----+-----+-------------+---+------+------+------+------+
+ * | sf | opc | 1 0 0 1 0 0 | N | immr | imms |  Rn  |  Rd  |
+ * +----+-----+-------------+---+------+------+------+------+
+ */
 static void disas_logic_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, opc, is_n, immr, imms, rn, rd;
+    TCGv_i64 tcg_rd, tcg_rn;
+    uint64_t wmask;
+    bool is_and = false;
+
+    sf = extract32(insn, 31, 1);
+    opc = extract32(insn, 29, 2);
+    is_n = extract32(insn, 22, 1);
+    immr = extract32(insn, 16, 6);
+    imms = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+
+    if (!sf && is_n) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    if (opc == 0x3) { /* ANDS */
+        tcg_rd = cpu_reg(s, rd);
+    } else {
+        tcg_rd = cpu_reg_sp(s, rd);
+    }
+    tcg_rn = cpu_reg(s, rn);
+
+    if (!logic_imm_decode_wmask(&wmask, is_n, imms, immr)) {
+        /* some immediate field values are reserved */
+        unallocated_encoding(s);
+        return;
+    }
+
+    if (!sf) {
+        wmask &= 0xffffffff;
+    }
+
+    switch (opc) {
+    case 0x3: /* ANDS */
+    case 0x0: /* AND */
+        tcg_gen_andi_i64(tcg_rd, tcg_rn, wmask);
+        is_and = true;
+        break;
+    case 0x1: /* ORR */
+        tcg_gen_ori_i64(tcg_rd, tcg_rn, wmask);
+        break;
+    case 0x2: /* EOR */
+        tcg_gen_xori_i64(tcg_rd, tcg_rn, wmask);
+        break;
+    default:
+        assert(FALSE); /* must handle all above */
+        break;
+    }
+
+    if (!sf && !is_and) {
+        /* zero extend final result; we know we can skip this for AND
+         * since the immediate had the high 32 bits clear.
+         */
+        tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+    }
+
+    if (opc == 3) { /* ANDS */
+        gen_logic_CC(sf, tcg_rd);
+    }
 }
 
 /* Move wide (immediate) */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 13/13] target-arm: A64: add support for logical (immediate) insns

[Richard Henderson]

On 12/06/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> This patch adds support for C3.4.4 Logical (immediate),
> which include AND, ANDS, ORR, EOR.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> [claudio: adapted to new decoder, function renaming,
>           removed a TCG temp variable]
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> [PMM: cleaned up some unnecessary code in logic_imm_decode_wmask
> and added clarifying commentary on what it's actually doing.
> Dropped an ext32u that's not needed if we've just done an AND.]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |  175 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 173 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~