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~