Re: [patch V2 16/17] lib/vdso: Allow architectures to override the ns shift operation

[Vincenzo Frascino]

On 07/02/2020 12:39, Thomas Gleixner wrote:
> From: Christophe Leroy <christophe.leroy@xxxxxx>
> 
> On powerpc/32, GCC (8.1) generates pretty bad code for the ns >>= vd->shift
> operation taking into account that the shift is always <= 32 and the upper
> part of the result is likely to be zero. GCC makes reversed assumptions
> considering the shift to be likely >= 32 and the upper part to be like not
> zero.
> 
> unsigned long long shift(unsigned long long x, unsigned char s)
> {
> 	return x >> s;
> }
> 
> results in:
> 
> 00000018 <shift>:
>   18:	35 25 ff e0 	addic.  r9,r5,-32
>   1c:	41 80 00 10 	blt     2c <shift+0x14>
>   20:	7c 64 4c 30 	srw     r4,r3,r9
>   24:	38 60 00 00 	li      r3,0
>   28:	4e 80 00 20 	blr
>   2c:	54 69 08 3c 	rlwinm  r9,r3,1,0,30
>   30:	21 45 00 1f 	subfic  r10,r5,31
>   34:	7c 84 2c 30 	srw     r4,r4,r5
>   38:	7d 29 50 30 	slw     r9,r9,r10
>   3c:	7c 63 2c 30 	srw     r3,r3,r5
>   40:	7d 24 23 78 	or      r4,r9,r4
>   44:	4e 80 00 20 	blr
> 
> Even when forcing the shift to be smaller than 32 with an &= 31, it still
> considers the shift as likely >= 32.
> 
> Move the default shift implementation into an inline which can be redefined
> in architecture code via a macro.
> 
> [ tglx: Made the shift argument u32 and removed the __arch prefix ]
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/b3d449de856982ed060a71e6ace8eeca4654e685.1580399657.git.christophe.leroy@xxxxxx
> 


Reviewed-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
Tested-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>

[...]
-- 
Regards,
Vincenzo

Attachment: pEpkey.asc
Description: application/pgp-keys