linux-kernel - Re: [PATCH 4/5] arch: sh: use sign

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	16 Oct 2015 17:25:51 -0400
From:	"George Spelvin" <linux@...izon.com>
To:	akpm@...ux-foundation.org, dvlasenk@...hat.com, linux@...izon.com,
	linux@...musvillemoes.dk, martink@...teo.de,
	maxime.coquelin@...com, mingo@...nel.org,
	torvalds@...ux-foundation.org, yury.norov@...il.com
Cc:	linux-kernel@...r.kernel.org,
	martin.kepplinger@...obroma-systems.com
Subject: Re: [PATCH 4/5] arch: sh: use sign_extend64() for sign extension

>  			/* Sign extend */
>  			regcache[dest] =
> -				((((s64)(u64)op >> 10) & 0xffff) << 54) >> 54;
> +				sign_extend64((((u64)op >> 10) & 0xffff), 9);
>  			break;

If you're cleaning up the code, cleaning it all the way up would be nice.

The mask wasn't necessary in the original, and the cast to (u64)
is unnecessary with sign_extend64.

So the first cleanup stage is
> +				sign_extend64(op >> 10, 9);

You can improve the 64-bit code by teaching GCC to combine the two
right shifts, but the 32-bit code is a disaster:
> +				sign_extend64(op, 19) >> 10;

And, for the benefit of 32-bit processors, you could just use the 32-bit
version and let the 32->64 bit sign extension happen automatically:
> +				sign_extend32(op, 19) >> 10;

Here are the 5 alternatives, fed through gcc -O3, in 32-bit and 64-bit code,
for x86 and ARM architectures:

	32-bit			64-bit
Original:
	sarl	$10, %eax		sarl	$10, %eax
	sall	$22, %eax		salq	$54, %rax
	cltd		 		sarq	$54, %rax
	sarl	$22, %eax

	mov	r0, r0, asr #10
	mov	r1, r0, asl #22
	mov	r0, r1, asr #22
	mov	r1, r1, asr #31

sign_extend64((((u64)op >> 10) & 0xffff), 9):
	sall	$12, %eax		salq	$44, %rax
	andl	$0xffc00000, %eax	sarq	$54, %rax
	cltd
	sarl	$22, %eax

	mov	r3, r0, asr #31		sbfx	x0, x0, 10, 10
	mov	r0, r0, lsr #10
	orr	r0, r0, r3, asl #22
	mov	r1, r0, asl #22
	mov	r0, r1, asr #22
	mov	r1, r1, asr #31

sign_extend64(op >> 10, 9):
	sarl	$10, %eax		sarl	$10, %eax
	sall	$22, %eax		salq	$54, %rax
	cltd				sarq	$54, %rax
	sarl	$22, %eax

	mov	r0, r0, asr #10		asr	w0, w0, 10
	mov	r1, r0, asl #22		sbfx	x0, x0, 0, 10
	mov	r0, r1, asr #22
	mov	r1, r1, asr #31

sign_extend64(op, 19) >> 10:
	sall	$12, %eax		salq	$44, %rax
	pushl	%ebx			sarq	$54, %rax
	movl	%eax, %ecx
	movl	%eax, %ebx
	sarl	$12, %ebx
	sarl	$31, %ecx
	movl	%ebx, %eax
	movl	%ecx, %edx
	shrdl	$10, %edx, %eax
	popl	%ebx
	sarl	$10, %edx

	mov	r1, r0, asl #12		sbfx	x0, x0, 10, 10
	mov	r0, r1, asr #12
	mov	r0, r0, lsr #10
	mov	r1, r1, asr #31
	orr	r0, r0, r1, asl #22


sign_extend32(op, 19) >> 10:
	sall	$12, %eax		sall	$12, %eax
	sarl	$22, %eax		sarl	$22, %eax
	cltq				cltq

	mov	r0, r0, asl #12		sbfx	x0, x0, 10, 10
	mov	r0, r0, asr #22
	mov	r1, r0, asr #31

	(Or if -march is high enough)
	sbfx	r0, r0, #10, #10
	mov	r1, r0, asr #31
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/