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

[Peter Maydell]

On 11 October 2016 at 16:51, Thomas Hanson <address@hidden> wrote:
> On 5 October 2016 at 16:01, Peter Maydell <address@hidden> wrote:
>> 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.

> This approach seems counter to both structured and OO design principles
> which would push common code (like type conversion) down into the lower
> level function in order to increase re-use and minimize code duplication.
> Those principles suggest that if we need a gen_a64_set_pc_value() function
> that can load the PC from something other than a register or an immediate,
> then it should be a lower level function than, and be called by,
> gen_a64_set_pc_reg().  This also has the benefit of reducing clutter in the
> caller, making it more readable and more maintainable.

The 'lower level' stuff here has a general pattern of taking either
(1) a TCGv or (2) an integer immediate. We should follow that pattern.

> As a separate issue, we now have functions to load the PC from an immediate
> value and from a register.  Where else could we legitimately load the PC
> from?

Anything where we found ourselves wanting to do some preliminary
manipulation of the value before writing it to the PC.

thanks
-- PMM