[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 12/12 v9] target-tilegx: Generate tcg instructio
From: |
Chen Gang |
Subject: |
Re: [Qemu-devel] [PATCH 12/12 v9] target-tilegx: Generate tcg instructions to execute to 1st system call |
Date: |
Sat, 11 Apr 2015 05:28:49 +0800 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 4/10/15 06:19, Peter Maydell wrote:
> On 27 March 2015 at 11:07, Chen Gang <address@hidden> wrote:
[...]
>>
>> +static void gen_fnop(void)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "(f)nop\n");
>
> I really don't much like mixing a fake disassembler in
> with the translator. If you want a disassembler, write one
> and put it in disas/...
>
For me, it is necessary (for I am not quite sure whether my disassemble
must be correct). It is only for tracing.
>> +}
>> +
>> +static void gen_cmpltui(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, int8_t imm8)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "cmpltui r%d, r%d, %d\n",
>> + rdst, rsrc, imm8);
>> + tcg_gen_setcondi_i64(TCG_COND_LTU, dest_gr(dc, rdst), load_gr(dc, rsrc),
>> + (uint64_t)imm8);
>> +}
>> +
>> +static void gen_cmpeqi(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, int8_t imm8)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "cmpeqi r%d, r%d, %d\n", rdst, rsrc,
>> imm8);
>> + tcg_gen_setcondi_i64(TCG_COND_EQ, dest_gr(dc, rdst), load_gr(dc, rsrc),
>> + (uint64_t)imm8);
>> +}
>> +
>> +static void gen_cmpne(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, uint8_t rsrcb)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "cmpne r%d, r%d, r%d\n",
>> + rdst, rsrc, rsrcb);
>> + tcg_gen_setcond_i64(TCG_COND_NE, dest_gr(dc, rdst), load_gr(dc, rsrc),
>> + load_gr(dc, rsrcb));
>> +}
>> +
>> +static void gen_cmoveqz(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, uint8_t rsrcb)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "cmoveqz r%d, r%d, r%d\n",
>> + rdst, rsrc, rsrcb);
>> + tcg_gen_movcond_i64(TCG_COND_EQ, dest_gr(dc, rdst), load_gr(dc, rsrc),
>> + load_zero(dc), load_gr(dc, rsrcb), load_gr(dc,
>> rdst));
>> +}
>> +
>> +static void gen_cmovnez(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, uint8_t rsrcb)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "cmovnez r%d, r%d, r%d\n",
>> + rdst, rsrc, rsrcb);
>> + tcg_gen_movcond_i64(TCG_COND_NE, dest_gr(dc, rdst), load_gr(dc, rsrc),
>> + load_zero(dc), load_gr(dc, rsrcb), load_gr(dc,
>> rdst));
>> +}
>
> This is hugely repetitive. Write a common function that takes a
> TCG_COND_* as a parameter.
>
OK, thanks.
>> +
>> +static void gen_add(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, uint8_t rsrcb)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "add r%d, r%d, r%d\n",
>> + rdst, rsrc, rsrcb);
>> + tcg_gen_add_i64(dest_gr(dc, rdst), load_gr(dc, rsrc), load_gr(dc,
>> rsrcb));
>> +}
>> +
>> +static void gen_addimm(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, int64_t imm)
>> +{
>> + tcg_gen_addi_i64(dest_gr(dc, rdst), load_gr(dc, rsrc), imm);
>> +}
>> +
>> +static void gen_addi(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, int8_t imm8)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "addi r%d, r%d, %d\n", rdst, rsrc,
>> imm8);
>> + gen_addimm(dc, rdst, rsrc, (int64_t)imm8);
>
> This cast is pointless, because the 'imm' argument to
> gen_addimm() already has type int64_t.
>
OK, thanks.
>> +}
>> +
>> +static void gen_addli(struct DisasContext *dc,
>> + uint8_t rdst, uint8_t rsrc, int16_t im16)
>> +{
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, "addli r%d, r%d, %d\n", rdst, rsrc,
>> im16);
>> + gen_addimm(dc, rdst, rsrc, (int64_t)im16);
>
> Another pointless cast.
>
OK, thanks.
[...]
>> +
>> +/*
>> + * The related functional description for bfextu in isa document:
>> + *
>> + * uint64_t mask = 0;
>> + * mask = (-1ULL) ^ ((-1ULL << ((BFEnd - BFStart) & 63)) << 1);
>> + * uint64_t rot_src = (((uint64_t) rf[SrcA]) >> BFStart)
>> + * | (rf[SrcA] << (64 - BFStart));
>> + * rf[Dest] = rot_src & mask;
>> + */
>> +static void gen_bfextu(struct DisasContext *dc, uint8_t rdst, uint8_t rsrc,
>> + int8_t start, int8_t end)
>> +{
>> + uint64_t mask = (-1ULL) ^ ((-1ULL << ((end - start) & 63)) << 1);
>> + TCGv tmp = dest_gr(dc, rdst);
>
> Are the start and end immediates here limited such that we're
> guaranteed not to hit any of C's undefined behaviour for out
> of range shifts, and that we don't hit TCG's undefined-value
> behaviour on bad rotates?
>
For me, it is correct, it is only the copy of the document, which has
already considered about any cases (include C's undefined behaviour).
>> static void decode_addi_opcode_y0(struct DisasContext *dc,
>> tilegx_bundle_bits bundle)
>> {
>> + uint8_t rsrc = (uint8_t)get_SrcA_Y0(bundle);
>> + uint8_t rdst = (uint8_t)get_Dest_Y0(bundle);
>> + int8_t imm8 = (int8_t)get_Imm8_Y0(bundle);
>
> These casts are all pointless.
>
OK, thanks. I shall change the return value of get_SrcA_Y0 and others in
opcode_tilegx.h.
[...]
>> @@ -127,8 +547,13 @@ static void decode_rrr_1_opcode_y0(struct DisasContext
>> *dc,
>> static void decode_rrr_5_opcode_y0(struct DisasContext *dc,
>> tilegx_bundle_bits bundle)
>> {
>> + uint8_t rsrc = (uint8_t)get_SrcA_Y0(bundle);
>> + uint8_t rsrcb = (uint8_t)get_SrcB_Y0(bundle);
>> + uint8_t rdst = (uint8_t)get_Dest_Y0(bundle);
>
> And again, and in many other places.
>
OK, thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed