[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 06/15] target-tricore: Add instructions of SR
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH v2 06/15] target-tricore: Add instructions of SRC opcode format |
Date: |
Mon, 14 Jul 2014 14:05:35 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 |
On 07/14/2014 10:41 AM, Bastian Koppelmann wrote:
> +target_ulong helper_shac(CPUTRICOREState *env, target_ulong r1,
> + target_ulong r2, target_ulong len)
> +{
> + target_ulong carry_out, msk, msk_start, msk_len, ret;
> + int32_t shift_count;
> + int const6;
> + const6 = sextract32(r2, 0, len);
> +
> + if (const6 >= 0) {
> + if (const6 != 0) {
> + msk_start = 32 - const6;
> + msk_len = 31-msk_start;
> + msk = ((1 << msk_len) - 1) << msk_start;
> + carry_out = ((r1 & msk) != 0);
> + } else {
> + carry_out = 0;
> + }
> + ret = r1 << const6;
> + } else {
> +
> + shift_count = 0 - const6;
> + ret = (int32_t)r1 >> shift_count;
> + msk = (1 << (shift_count - 1)) - 1;
> + carry_out = ((r1 & msk) != 0);
> + }
> + if (carry_out) {
> + /* TODO: carry out */
> + }
> + return ret;
> +}
Why a helper for SHA? It's not any more difficult than SH.
> +static void gen_shi(TCGv ret, TCGv r1, int32_t r2, int len)
> +{
> +/* shift_count = sign_ext(const4[3:0]);
> + D[a] = (shift_count >= 0) ? D[a] << shift_count : D[a] >> (-shift_count);
> */
> + int shift_count = sextract32(r2, 0, len);
Careful with your documentation: you're adding the 16-bit documentation as
opposed to the more generic 32-bit documentation.
I do not think you should have a "len" parameter at all. We've already
sign-extended const4 in decode_src_opc, so there's no need to do it again.
> +static void gen_shaci(TCGv ret, TCGv r1, int32_t con, int len)
> +{
> + TCGv temp = tcg_const_i32(con);
> +
> + gen_shac(ret, r1, temp, len);
> +
> + tcg_temp_free(temp);
> +}
In particular, SHACI with an immediate is pretty much exactly SHI except with
an arithmetic right shift instead of a logical right shift.
(Yes, there's some carry and overflow bit computation to do, but it's not like
you've implemented any of that in your current implementation either.)
> +
> +/*
> + * Functions for decoding instructions
> + */
> +
> +static void decode_src_opc(DisasContext *ctx, int op1)
> +{
> + int r1;
> + int32_t const4;
> + TCGv temp, temp2;
> +
> + r1 = MASK_OP_SRC_S1D(ctx->opcode);
> + const4 = MASK_OP_SRC_CONST4_SEXT(ctx->opcode);
> +
> + switch (op1) {
> +
> + case OPC1_16_SRC_ADD:
Watch the silly blank likes, above the case. And the end-of-file blank lines
in some of the other patches.
> + tcg_gen_addi_tl(cpu_gpr_d[r1], cpu_gpr_d[r1], const4);
Are you planning to come back to implement V and AV bits later?
> + case OPC1_16_SRC_MOV_A:
> + /* load const4 again unsigned */
> + const4 = MASK_OP_SRC_CONST4(ctx->opcode);
> + tcg_gen_movi_tl(cpu_gpr_a[r1], const4);
Err.. I don't think this is right. I see "signed" on page 3-224.
> + case OPC1_16_SRC_SHA:
> + /* FIXME: const too long */
> + gen_shaci(cpu_gpr_d[r1], cpu_gpr_d[r1], const4, 4);
> + break;
Huh? Why the fixme?
r~
- [Qemu-devel] [PATCH v2 00/15] TriCore architecture guest implementation, Bastian Koppelmann, 2014/07/14
- [Qemu-devel] [PATCH v2 04/15] target-tricore: Add initialization for translation and activate target, Bastian Koppelmann, 2014/07/14
- [Qemu-devel] [PATCH v2 02/15] target-tricore: Add board for systemmode, Bastian Koppelmann, 2014/07/14
- [Qemu-devel] [PATCH v2 05/15] target-tricore: Add masks and opcodes for decoding, Bastian Koppelmann, 2014/07/14
- [Qemu-devel] [PATCH v2 13/15] target-tricore: Add instructions of SC opcode format, Bastian Koppelmann, 2014/07/14
- [Qemu-devel] [PATCH v2 07/15] target-tricore: Add instructions of SRR opcode format, Bastian Koppelmann, 2014/07/14