On Fri, Jun 30, 2017 at 12:47 AM, Richard Henderson <r...@twiddle.net> wrote:
> On 06/29/2017 05:40 PM, Pranith Kumar wrote:
>>
>> void aarch64_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr)
>> {
>> tcg_insn_unit *code_ptr = (tcg_insn_unit *)jmp_addr;
>> - tcg_insn_unit *target = (tcg_insn_unit *)addr;
>> + tcg_insn_unit i1, i2;
>> + uint64_t pair;
>> + ptrdiff_t offset = addr - jmp_addr;
>> +
>> + if (offset == sextract64(offset, 0, 26)) {
>> + i1 = NOP;
>> + i2 = I3206_B | ((offset >> 2) & 0x3ffffff);
>
>
> Branch first, since that's the offset you calculated.
> Also, the nop need not be executed.
This is exactly how I form the instruction pair below (B+NOP, not
NOP+B). But I get your point. It is confusing to use i1 for the second
instruction. I'll change it.
>
>> + } else {
>> + offset = (addr >> 12) - (jmp_addr >> 12);
>> +
>> + /* patch ADRP */
>> + i2 = deposit32(*code_ptr++, 29, 2, offset & 0x3);
>> + i2 = deposit32(i2, 5, 19, offset >> 2);
>> + /* patch ADDI */
>> + i1 = deposit32(*code_ptr, 10, 12, addr & 0xfff);
>
>
> You can't just patch these insns, because they aren't necessarily ADRP+ADD.
> Indeed, they will very likely be B and NOP. The first address we patch in
> is tb_jmp_reset_offset, which is the following opcode, which is definitely
> in range of the branch above.
Whoops, I totally missed that we patch these out the first time out. I
will explicitly generate the ADRP+ADD pair from here.
Thanks,
--
Pranith
