Re: [RFC][PATCH] Reduce generated code by 3% by increasing MMU indices

[Helge Deller <deller@gmx.de>]

On 8/5/23 21:40, Helge Deller wrote:
> On 8/5/23 19:58, Richard Henderson wrote:
>> On 8/5/23 10:43, Helge Deller wrote:
>>>> If there were a way to change no more than two lines of code,
>>>> that would be fine.  But otherwise I don't see this as being
>>>> worth making the rest of the code base any more complex.
>>>
>>> Ok. What about that 6-line patch below for x86? It's trivial and
>>> all what's needed for x86. Btw, any index which is >= 9 will use
>>> the shorter code sequence.
>>>
>>> Helge
>>>
>>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h index
>>> e0771a1043..3e71e666db 100644 --- a/target/i386/cpu.h +++
>>> b/target/i386/cpu.h @@ -2251,11 +2251,11 @@ uint64_t
>>> cpu_get_tsc(CPUX86State *env); #define cpu_list x86_cpu_list
>>>
>>> /* MMU modes definitions */ -#define MMU_KSMAP_IDX   0 -#define
>>> MMU_USER_IDX    1 -#define MMU_KNOSMAP_IDX 2 -#define
>>> MMU_NESTED_IDX  3 -#define MMU_PHYS_IDX    4 +#define MMU_KSMAP_IDX
>>> 11 +#define MMU_USER_IDX    12 +#define MMU_KNOSMAP_IDX 13 +#define
>>> MMU_NESTED_IDX  14 +#define MMU_PHYS_IDX    15
>>
>> No.  The small patch would need to apply to all guests.
>
> Yes.
>
>> Perhaps something to handle indexing of CPUTLBDescFast, e.g.
>>
>> static inline CPUTLBDescFast cputlb_fast(CPUTLB *tlb, unsigned idx)
>> { return &tlb->f[NB_MMU_MODES - 1 - idx]; }
>>
>> There's already tlb_mask_table_ofs, which handles all tcg backends;
>> you just need to adjust that and cputlb.c> Introduce cputlb_fast with
>> normal indexing in one patch, and then the second patch to invert the
>> indexing may well be exactly two lines.  :-)
>
> You're cheating :-)
> But ok, that's an easy one and I can come up with both patches.
>
> One last idea which came into my mind and which may be worth
> asking before I start to hack the patch above...:
>
> include/exec/cpu-defs.h:
> /* add some comment here why we use this transformation: */
> #define MMU_INDEX(nr)    (NB_MMU_MODES - 1 - (x))
>
> target/*/cpu.h:
> /* MMU modes definitions */
> #define MMU_KSMAP_IDX   MMU_INDEX(0)
> #define MMU_USER_IDX    MMU_INDEX(1)
> #define MMU_KNOSMAP_IDX MMU_INDEX(2)
> #define MMU_NESTED_IDX  MMU_INDEX(3)
> ...
>
> Downside:
> - of course it's a lot more than the 2 lines you asked for
> Upsides:
> - no additional subtaction at tcg compile time/runtime
> - clear indication that this is an MMU index, easy to grep.
> - easy to use

and it's actually a 1-line patch as you requested :-)
   similiar to your approach above (multiple preparation patches,
   one last patch which just changes
#define MMU_INDEX(nr)    (nr)
   to
#define MMU_INDEX(nr)    (NB_MMU_MODES - 1 - (nr))

;-)

Helge