On 09/29/2016 07:24 PM, Peter Maydell wrote: > On 16 September 2016 at 10:34, Thomas Hanson <thomas.han...@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().
