qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/6 v6] target-tilegx: Firstly add TILE-Gx with


From: Chen Gang
Subject: Re: [Qemu-devel] [PATCH 1/6 v6] target-tilegx: Firstly add TILE-Gx with minimized features
Date: Thu, 19 Mar 2015 10:04:37 +0800
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

On 3/19/15 04:06, Richard Henderson wrote:
> On 03/18/2015 09:34 AM, Chen Gang wrote:
>> +static void gen_fnop(void)
>> +{
>> +    qemu_log_mask(CPU_LOG_TB_IN_ASM, "(f)nop\n");
>> +}
>> +
>> +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);
>> +}
> 
> Wow, this is a lot more than before.  Good progress.
> 

OK, thanks. Because I am hunting the new jobs during these days, so I
have more time resources on open source (at least, within this month).
:-)


>> +/*
>> + * 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)
>> +{
>> +    TCGv mask = tcg_temp_new_i64();
>> +    TCGv tmp = dest_gr(dc, rdst);
>> +
>> +    qemu_log_mask(CPU_LOG_TB_IN_ASM, "bfextu r%d, r%d, %d, %d\n",
>> +                  rdst, rsrc, start, end);
>> +
>> +    tcg_gen_movi_i64(tmp, -1ULL);
>> +    tcg_gen_movi_i64(mask, end);
>> +
>> +    tcg_gen_subi_i64(mask, mask, start);
>> +    tcg_gen_andi_i64(mask, mask, 63);
>> +    tcg_gen_shl_i64(mask, tmp, mask);
>> +    tcg_gen_shli_i64(mask, mask, 1);
>> +    tcg_gen_xori_i64(mask, mask, -1ULL);
> 
> This computation of MASK is only dependent on START and END, which are known 
> at
> translation time.  Thus you should perform this computation at translation 
> time
> here in C, rather than defer the computation with TCG opcodes.
> 
>> +    tcg_gen_rotli_i64(tmp, load_gr(dc, rsrc), start);
>> +    tcg_gen_and_i64(tmp, tmp, mask);
> 
> Which then makes this tcg_gen_andi_i64.
> 

OK, thanks, really it is.


>> +static void gen_mulx(struct DisasContext *dc,
>> +                     uint8_t rdst, uint8_t rsrc, uint8_t rsrcb)
>> +{
>> +    TCGv tmp = tcg_temp_new_i64();
>> +    TCGv vdst = dest_gr(dc, rdst);
>> +
>> +    qemu_log_mask(CPU_LOG_TB_IN_ASM, "mulx r%d, r%d, r%d\n", rdst, rsrc, 
>> rsrcb);
>> +
>> +    tcg_gen_ext32s_i64(vdst, load_gr(dc, rsrc));
>> +    tcg_gen_ext32s_i64(tmp, load_gr(dc, rsrcb));
>> +
>> +    tcg_gen_mul_i64(tmp, vdst, tmp);
>> +    tcg_gen_ext32s_i64(vdst, tmp);
> 
> Note that you don't need the extensions prior to the MUL.  The high 32-bits of
> the inputs don't affect the low 32-bits of the product.
>

For me, I am not quite sure about it, the related functional description
is:

  rf[Dest] = signExtend32 ((int32_t) rf[SrcA] * (int32_t) rf[SrcB]);

Do you mean it is equal to:

  rf[Dest] = signExtend32 (rf[SrcA] * rf[SrcB]);


>> +static void gen_shl16insli(struct DisasContext *dc,
>> +                           uint8_t rdst, uint8_t rsrc, uint16_t uimm16)
>> +{
>> +    TCGv vdst = dest_gr(dc, rdst);
>> +
>> +    qemu_log_mask(CPU_LOG_TB_IN_ASM, "shl16insli r%d, r%d, %llx\n",
>> +                  rdst, rsrc, (long long)uimm16);
> 
> Do not cast to long long.  Print with just %x.
>

OK, thanks.
 
>> +static int gen_beqz(struct DisasContext *dc, uint8_t rsrc, int32_t off)
>> +{
>> +    qemu_log_mask(CPU_LOG_TB_IN_ASM, "beqz(t) r%d, %d\n", rsrc, off);
>> +
>> +    dc->jmp.dest = tcg_temp_new_i64();
>> +    dc->jmp.val1 = tcg_temp_new_i64();
>> +    dc->jmp.val2 = tcg_temp_new_i64();
>> +
>> +    dc->jmp.cond = TCG_COND_EQ;
>> +    tcg_gen_movi_i64(dc->jmp.dest,
>> +                     dc->pc + (int64_t)off * TILEGX_BUNDLE_SIZE_IN_BYTES);
> 
> It will be helpful for the disassembly if you print the destination of the
> branch rather than the offset.
> 

OK, thanks.

>> +    qemu_log_mask(LOG_UNIMP, "UNIMP rrr_1_opcode_y0, %16.16llx\n", bundle);
> 
> You shouldn't use "llx".  You need to use PRIx64 instead.  This will enable
> proper operation on Windows.
> 

OK, thanks.


> This patch set is just about ready for approval (though I don't imagine it 
> will
> be merged until the qemu 2.4 development cycle).
> 

OK, thanks. And I shall try to send patch v7 for the left issues within
2 days (2015-03-20).


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed



reply via email to

[Prev in Thread] Current Thread [Next in Thread]