Re: [Qemu-devel] [PATCH 26/60] AArch64: Add ADR instruction emulation

[Claudio Fontana]

On 11/19/2013 06:17 PM, Claudio Fontana wrote:
> Hello all,
> 
> On 09/27/2013 02:48 AM, Alexander Graf wrote:
>> This patch adds emulation support for the adr instruction.
>>
>> Signed-off-by: Alexander Graf <address@hidden>
>> ---
>>  target-arm/translate-a64.c | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
>> index bc91324..00eda0f 100644
>> --- a/target-arm/translate-a64.c
>> +++ b/target-arm/translate-a64.c
>> @@ -943,6 +943,27 @@ static void handle_insg(DisasContext *s, uint32_t insn)
>>      simd_st(cpu_reg(rn), freg_offs_d + idx, size);
>>  }
>>  
>> +/* PC relative address calculation */
>> +static void handle_adr(DisasContext *s, uint32_t insn)
>> +{
>> +    int reg = get_reg(insn);
>> +    int is_page = get_bits(insn, 31, 1);
>> +    uint64_t imm;
>> +    uint64_t base;
>> +
>> +    imm = get_sbits(insn, 5, 19) << 2;
>> +    imm |= get_bits(insn, 29, 2);
>> +
>> +    base = s->pc - 4;
>> +    if (is_page) {
>> +        /* ADRP (page based) */
>> +        base &= ~0xFFFULL;
>> +        imm <<= 12;
>> +    }
>> +
>> +    tcg_gen_movi_i64(cpu_reg(reg), base + imm);
>> +}
>> +
> 
> does this work with negative values?
> The spec says to SignExtend:
> 
> if page then
>     imm = SignExtend(immhi:immlo:Zeros(12), 64);
> else
>     imm = SignExtend(immhi:immlo, 64);
> 
> /*...*/
> 
> maybe Michael you know if this is an issue in practice?
> If I want to get a negative PC relative offset, how does this work?
> 
> Claudio


This is a totally untested idea of what I would think/roughly implement:

static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
{
    /*
     * 31 30 29 28 27 26 25 24 23     5 4  0
     * op immlo  1  0  0  0  0   immhi   Rd
     */
    unsigned int page, imm, rd, len; /* op -> page, immhi:immlo -> imm */
    uint64_t base;
    sint64_t offset; /* SignExtend(imm) -> offset */

    page = insn & (1 << 31) ? 1 : 0;
    imm = extract32(insn, 29, 2) + extract32(insn, 5, 19) << 2;
    rd = extract32(insn, 0, 5);

    base = s->pc - 4;
    len = 19 + 2; /* immhi:immlo */
    offset = imm;

    if (page) {
        /* ADRP (page based) */
        base &= ~0xfff;
        len += 12; /* immhi:immlo:Zeros(12) */
        offset <<= 12; /* apply Zeros */
    }

    offset = (offset << (64 - len)) >> (64 - len); /* sign extend */
    tcg_gen_movi_i64(cpu_reg(reg), base + offset);
}

But maybe I am completely off and the original code is perfectly fine..?

C.

> 
> 
>>  /* SIMD ORR */
>>  static void handle_simdorr(DisasContext *s, uint32_t insn)
>>  {
>> @@ -1365,6 +1386,9 @@ void disas_a64_insn(CPUARMState *env, DisasContext *s)
>>              unallocated_encoding(s);
>>          }
>>          break;
>> +    case 0x10:
>> +        handle_adr(s, insn);
>> +        break;
>>      default:
>>          unallocated_encoding(s);
>>          break;
>>
>