On 23 May 2013 09:18, Claudio Fontana <claudio.font...@huawei.com> 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.)
> +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).
> +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.
> + 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).
thanks
-- PMM
