diff mbox

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

Message ID 1386280289-27636-6-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell Dec. 5, 2013, 9:51 p.m. UTC 
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(-)

Comments

Richard Henderson Dec. 5, 2013, 10:51 p.m. UTC | #1 
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~
Peter Maydell Dec. 5, 2013, 11:09 p.m. UTC | #2 
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
Richard Henderson Dec. 5, 2013, 11:13 p.m. UTC | #3 
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~
Claudio Fontana Dec. 5, 2013, 11:21 p.m. UTC | #4 
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
>
Eric Blake Dec. 5, 2013, 11:24 p.m. UTC | #5 
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.
diff mbox

Patch

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 */