From: Gabriel Paubert <paubert@iram•es>
To: Madhavan Srinivasan <maddy@linux•vnet.ibm.com>
Cc: rusty@rustcorp•com.au, paulus@samba•org, anton@samba•org,
linuxppc-dev@lists•ozlabs.org
Subject: Re: [RFC PATCH 1/2]powerpc: foundation code to handle CR5 for local_t
Date: Wed, 3 Dec 2014 18:07:43 +0100 [thread overview]
Message-ID: <20141203170743.GA11062@visitor2.iram.es> (raw)
In-Reply-To: <547F2559.9060404@linux.vnet.ibm.com>
On Wed, Dec 03, 2014 at 08:29:37PM +0530, Madhavan Srinivasan wrote:
> On Tuesday 02 December 2014 03:05 AM, Gabriel Paubert wrote:
> > On Thu, Nov 27, 2014 at 05:48:40PM +0530, Madhavan Srinivasan wrote:
> >> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> >> index 77f52b2..c42919a 100644
> >> --- a/arch/powerpc/include/asm/exception-64s.h
> >> +++ b/arch/powerpc/include/asm/exception-64s.h
> >> @@ -306,7 +306,26 @@ do_kvm_##n: \
> >> std r10,0(r1); /* make stack chain pointer */ \
> >> std r0,GPR0(r1); /* save r0 in stackframe */ \
> >> std r10,GPR1(r1); /* save r1 in stackframe */ \
> >> - beq 4f; /* if from kernel mode */ \
> >> +BEGIN_FTR_SECTION; \
> >> + lis r9,4096; /* Create a mask with HV and PR */ \
> >> + rldicr r9,r9,32,31; /* bits, AND with the MSR */ \
> >> + mr r10,r9; /* to check for Hyp state */ \
> >> + ori r9,r9,16384; \
> >> + and r9,r12,r9; \
> >> + cmpd cr3,r10,r9; \
> >> + beq cr3,66f; /* Jump if we come from Hyp mode*/ \
> >> + mtcrf 0x04,r10; /* Clear CR5 if coming from usr */ \
> >
> > I think you can do better than this, powerpc has a fantastic set
> > of rotate and mask instructions. If I understand correctly your
> > code you can replace it with the following:
> >
> > rldicl r10,r12,4,63 /* Extract HV bit to LSB of r10*/
> > rlwinm r9,r12,19,0x02 /* Extract PR bit to 2nd to last bit of r9 */
> > or r9,r9,10
> > cmplwi cr3,r9,1 /* Check for HV=1 and PR=0 */
> > beq cr3,66f
> > mtcrf 0x04,r10 /* Bits going to cr5 bits are 0 in r10 */
> >
>
> >From previous comment, at this point, CR0 already has MSR[PR], and only
> HV need to be checked. So I guess there still room for optimization.
> But good to learn this seq.
Actually the sequence I suggested can even be shortened again since the or
is not necessary and you can use an rlwimi instead.
rldicl r9,r12,5,62 /* r9 = 62 zeroes | HV | ? */
rlwimi r9,r12,18,0x01 /* r9 = 62 zeroes | HV | PR */
cmplwi cr3,r9,2 /* Check for HV=1 and PR=0 */
For 32 bit operands, rlwimi is a generic bit field to bit field move, but
GCC is (was, maybe GCC5 is better?) quite bad at recognizing opportunities
to use it.
I'm not sure that it is better to have two separate branches, one for
testing PR and the other for testing HV. Through how many branches can
the hardware speculate?
Unless I'm mistaken, this is code which is likely not to be in the
cache so I would optimize it first towards minimal code size.
Regards,
Gabriel
next prev parent reply other threads:[~2014-12-03 17:08 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-27 12:18 [RFC PATCH 0/2] powerpc: CR based local atomic operation implementation Madhavan Srinivasan
2014-11-27 12:18 ` [RFC PATCH 1/2]powerpc: foundation code to handle CR5 for local_t Madhavan Srinivasan
2014-11-27 16:56 ` Segher Boessenkool
2014-11-28 1:58 ` Benjamin Herrenschmidt
2014-11-28 3:00 ` Madhavan Srinivasan
2014-11-28 15:57 ` Segher Boessenkool
2014-11-28 2:57 ` Madhavan Srinivasan
2014-11-28 16:00 ` Segher Boessenkool
2014-11-28 0:56 ` Benjamin Herrenschmidt
2014-11-28 3:15 ` Madhavan Srinivasan
2014-11-28 3:21 ` Benjamin Herrenschmidt
2014-11-28 10:53 ` David Laight
2014-11-30 9:01 ` Benjamin Herrenschmidt
2014-12-01 21:35 ` Gabriel Paubert
2014-12-03 14:59 ` Madhavan Srinivasan
2014-12-03 17:07 ` Gabriel Paubert [this message]
2014-12-02 2:04 ` Scott Wood
2014-12-03 14:49 ` Madhavan Srinivasan
2014-11-27 12:18 ` [RFC PATCH 2/2]powerpc: rewrite local_* to use CR5 flag Madhavan Srinivasan
2014-12-01 18:01 ` Gabriel Paubert
2014-12-03 15:05 ` Madhavan Srinivasan
2014-11-27 14:05 ` [RFC PATCH 0/2] powerpc: CR based local atomic operation implementation David Laight
2014-11-28 8:27 ` Madhavan Srinivasan
2014-11-28 10:09 ` David Laight
2014-12-01 15:35 ` Madhavan Srinivasan
2014-12-01 15:53 ` David Laight
2014-12-18 4:18 ` Rusty Russell
2014-12-18 9:52 ` David Laight
2014-12-18 10:53 ` Rusty Russell
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=20141203170743.GA11062@visitor2.iram.es \
--to=paubert@iram$(echo .)es \
--cc=anton@samba$(echo .)org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=maddy@linux$(echo .)vnet.ibm.com \
--cc=paulus@samba$(echo .)org \
--cc=rusty@rustcorp$(echo .)com.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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