On 11 October 2016 at 16:51, Thomas Hanson <thomas.han...@linaro.org> wrote: > On 5 October 2016 at 16:01, Peter Maydell <peter.mayd...@linaro.org> 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
