Re: [PATCH 36/43] target/ppc/mmu-hash32: Remove some static inlines from header

[BALATON Zoltan]

On Mon, 8 Jul 2024, Nicholas Piggin wrote:
> On Sun Jul 7, 2024 at 6:18 AM AEST, BALATON Zoltan wrote:
> > On Thu, 4 Jul 2024, Nicholas Piggin wrote:
> > > On Mon May 27, 2024 at 9:13 AM AEST, BALATON Zoltan wrote:
> > > > Two of these are not used anywhere and the other two are used only
> > > > once and can be inlined and removed from the header.
> > >
> > > I'd prefer to put these in the .c file. Probably calculating the
> > > base once would generate marginally better code since it would not
> > > have to keep reloading it (since there is a barrier there it can't
> > > cache the value).
> >
> > These aren't even used anywhere but one function and they are inefficient
> > becuase they would call ppc_hash32_hpt_base() on each call. Next patch
> > even removes base and calculates pte_addr once before the loop, then it's
> > quite straight forward what these read from guest memory even without
> > inline functions. I see no reason to keep these inline functions.
>
> Make them take the hash base instead of cpu if that's the performance
> issue to solve. And open coded access can always be converted to use
> it.

If you look at the next patch you can see the base calculatoin is gone and 
it does pte_addr = ppc_hash32_hpt_base(cpu) + pteg_off once at the 
beginning before the loop, then the function you want to make is just: 
pte0 = ldl_phys(CPU(cpu)->as, pte_addr). I don't think it's worth making 
it a separate fucntion .The other one
pte1 = ldl_phys(CPU(cpu)->as, pte_addr + HASH_PTE_SIZE_32 / 2)
still has some calculation left but that's pretty straight forward. Maybe 
we could add a mcacro for HASH_PTE_SIZE_32 / 2 like HARH_PTE1_OFFS or 
something but I don't think that or having separate functions for this 
would add any more clarity just unnecessary complication. Yout one line 
helpers would only be used by ppc_hash32_pteg_search which is already a 
helper tp get pte values so open coding it within this function is OK in 
my opinion. There are no other places your helper functions would be 
needed.

Regards,
BALATON Zoltlan