mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "George Spelvin" <linux@horizon.com>
To: akpm@linux-foundation.org, dvlasenk@redhat.com,
	linux@horizon.com, linux@rasmusvillemoes.dk, martink@posteo.de,
	maxime.coquelin@st.com, mingo@kernel.org,
	torvalds@linux-foundation.org, yury.norov@gmail.com
Cc: linux-kernel@vger.kernel.org, martin.kepplinger@theobroma-systems.com
Subject: Re: [PATCH 4/5] arch: sh: use sign_extend64() for sign extension
Date: 16 Oct 2015 17:25:51 -0400	[thread overview]
Message-ID: <20151016212551.21817.qmail@ns.horizon.com> (raw)
In-Reply-To: <1444986427-12101-5-git-send-email-martink@posteo.de>

>  			/* 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

  reply	other threads:[~2015-10-16 21:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16  9:07 [PATCH v2 0/5] improve sign extension API Martin Kepplinger
2015-10-16  9:07 ` [PATCH 1/5] bitops.h: Improve sign_extend32()'s documentation Martin Kepplinger
2015-10-16  9:07 ` [PATCH 2/5] bitops.h: add sign_extend64() Martin Kepplinger
2015-10-16  9:07 ` [PATCH 3/5] arch: sh: use sign_extend64() for sign extension Martin Kepplinger
2015-10-16  9:07 ` [PATCH 4/5] " Martin Kepplinger
2015-10-16 21:25   ` George Spelvin [this message]
2015-10-16  9:07 ` [PATCH 5/5] arch: x86: " Martin Kepplinger
2015-10-16 21:30   ` George Spelvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151016212551.21817.qmail@ns.horizon.com \
    --to=linux@horizon.com \
    --cc=akpm@linux-foundation.org \
    --cc=dvlasenk@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=martin.kepplinger@theobroma-systems.com \
    --cc=martink@posteo.de \
    --cc=maxime.coquelin@st.com \
    --cc=mingo@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®