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

[Peter Maydell]

On 5 October 2016 at 14:53, Tom Hanson <address@hidden> wrote:
> On 09/29/2016 07:24 PM, Peter Maydell wrote:
>> On 16 September 2016 at 10:34, Thomas Hanson <address@hidden> wrote:
>>> +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().

It matches the style of the rest of the code which generally
prefers to convert register numbers into TCGv earlier rather
than later (at the level which is doing decode of instruction
bits, rather than inside utility functions), and gives you a
more flexible utility function, which can do a "write value to PC"
for any value, not just something that happens to be in a CPU
register. And as you say it avoids calling cpu_reg() multiple times
as a side benefit.

thanks
-- PMM