[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for a
From: |
Claudio Fontana |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64 |
Date: |
Fri, 24 May 2013 10:51:18 +0200 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6 |
On 23.05.2013 18:39, Peter Maydell wrote:
> On 23 May 2013 09:18, Claudio Fontana <address@hidden> wrote:
>>
>> add preliminary support for TCG target aarch64.
>
> Richard's handling the technical bits of the review, so
> just some minor style nits here.
>
> I tested this on the foundation model and was able to boot
> a 32-bit-ARM kernel.
>
>> +static inline void reloc_pc19(void *code_ptr, tcg_target_long target)
>> +{
>> + tcg_target_long offset; uint32_t insn;
>> + offset = (target - (tcg_target_long)code_ptr) / 4;
>> + offset &= 0x07ffff;
>> + /* read instruction, mask away previous PC_REL19 parameter contents,
>> + set the proper offset, then write back the instruction. */
>> + insn = *(uint32_t *)code_ptr;
>> + insn = (insn & 0xff00001f) | offset << 5; /* lower 5 bits = condition */
>
> You can say
> insn = deposit32(insn, 5, 19, offset);
> here rather than doing
> offset &= 0x07ffff;
> insn = (insn & 0xff00001f) | offset << 5;
>
> (might as well also use deposit32 for consistency in the pc26 function.)
Ok, I'll make use of it.
>> +static inline enum aarch64_ldst_op_data
>> +aarch64_ldst_get_data(TCGOpcode tcg_op)
>> +{
>> + switch (tcg_op) {
>> + case INDEX_op_ld8u_i32: case INDEX_op_ld8s_i32:
>> + case INDEX_op_ld8u_i64: case INDEX_op_ld8s_i64:
>> + case INDEX_op_st8_i32: case INDEX_op_st8_i64:
>
> One case per line, please (here and elsewhere).
Will comply.
>> +static inline void tcg_out_call(TCGContext *s, tcg_target_long target)
>> +{
>> + tcg_target_long offset;
>> +
>> + offset = (target - (tcg_target_long)s->code_ptr) / 4;
>> +
>> + if (offset <= -0x02000000 || offset >= 0x02000000) { /* out of 26bit
>> rng */
>> + tcg_out_movi64(s, TCG_REG_TMP, target);
>> + tcg_out_callr(s, TCG_REG_TMP);
>> +
>
> Stray blank line.
I should remove this \n I assume. Ok.
>> + case INDEX_op_mov_i64: ext = 1;
>
> Please don't put code on the same line as a case statement.
> Also fall-through cases should have an explicit /* fall through */
> comment (except in the case where there is no code at all
> between one case statement and the next).
Will change for the next version.
> thanks
> -- PMM
>
Thank you,
Claudio
- Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, (continued)
- Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Claudio Fontana, 2013/05/27
- Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Richard Henderson, 2013/05/27
- Re: [Qemu-devel] [PATCH 3/3] tcg/aarch64: implement new TCG target for aarch64, Laurent Desnogues, 2013/05/27
- Re: [Qemu-devel] [PATCH 3/3] tcg/aarch64: implement new TCG target for aarch64, Claudio Fontana, 2013/05/28
- Re: [Qemu-devel] [PATCH 3/3] tcg/aarch64: implement new TCG target for aarch64, Laurent Desnogues, 2013/05/28
- Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Claudio Fontana, 2013/05/28
- Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Richard Henderson, 2013/05/28
Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Peter Maydell, 2013/05/23
Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Laurent Desnogues, 2013/05/27
Re: [Qemu-devel] [PATCH 2/4] tcg/aarch64: implement new TCG target for aarch64, Laurent Desnogues, 2013/05/28
Re: [Qemu-devel] [PATCH 0/4] ARM aarch64 TCG target VERSION 2, Andreas Färber, 2013/05/23