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

[Richard Henderson <richard.henderson@linaro.org>]

On 8/5/23 13:04, Helge Deller wrote:
> 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))
> 
> ;-)

:-) Plausible.  With a go, anyway.

r~