Re: [Qemu-devel] [PATCH 2/3] target-arm: Code changes to implement overwrite of tag field on PC load

[Tom Hanson <thomas.hanson@linaro.org>]

On 09/29/2016 07:24 PM, Peter Maydell wrote:
> On 16 September 2016 at 10:34, Thomas Hanson <thomas.hanson@linaro.org> wrote:
...
>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
>> index f5e29d2..4d6f951 100644
...
>> @@ -176,6 +177,58 @@ void gen_a64_set_pc_im(uint64_t val)
>>      tcg_gen_movi_i64(cpu_pc, val);
>>  }
>>
>> +void gen_a64_set_pc_reg(DisasContext *s, unsigned int rn)
> 
> I think it would be better to take a TCGv_i64 here rather than
> unsigned int rn (ie have the caller do the cpu_reg(s, rn)).
> (You probably don't need that prototype of cpu_reg() above if
> you do this, though that's not why it's better.)
> 
Why would this be better? 

To me, the caller has a register number and wants that register used to load 
the PC.  So, it passes in the register number.  

The fact that gen_a64_set_pc_reg() needs to convert that into a TCGv_i64 is
an implementation detail that should be encapsulated/hidden from the caller.

If the desire is to eliminate the multiple cpu_reg() calls inside of 
gen_a64_set_pc_reg() then that mapping could be done at the top of the 
function before the outer if().