[PATCH] Handle mtsprg and mfsprg properly for BookE
Jeff Baker
jbaker@qnx.com
Tue Mar 8 16:30:00 GMT 2005
> Correct for what processor? My BOOKE reference says of SPRG:
>
> SPR number Access Privileged
> SPRG0 272 Read/Write Yes
> SPRG1 273 Read/Write Yes
> SPRG2 274 Read/Write Yes
> SPRG3 259 Read-only No
> 275 Read/Write Yes
> SPRG4 260 Read-only No
> 276 Read/Write Yes
> SPRG5 261 Read-only No
> 277 Read/Write Yes
> SPRG6 262 Read-only No
> 278 Read/Write Yes
> SPRG7 263 Read-only No
> 279 Read/Write Yes
> USPRG0 256 Read/Write No
>
> That says to me that there are two possible SPR numbers you use to
> access each of SPRG3 to SPRG7. And the duplicate numbers start at
> SPRG3, not SPRG4. Now you know why nobody reviewed your patch, and
> why I'm not approving it as is..
My BookE reference states that user-mode read access to sprg3 is
implementation dependant. Since the opcode table doesn't have an
mfsprg3 with an SPR number of 259 I (apparently incorrectly) assumed
that we didn't need that.
I also assumed that one reason for submitting a patch to the mailing
list is so that people who know more than I do can review it and then
point out where I'm making mistakes. If that isn't the case then can
you suggest somewhere I can go or someone I can ask for help if I'm
working on a patch that I need but don't 100% understand?
> If I was a programmer with an ounce of sense (debatable), I'd want
> to use "mfspr gpr,sprgN" where the sprgN are symbols or macros equated
> to the right spr register for the context, rather than trusting the
> assembler to do the right thing with "mfsprg gpr,N" or even worse,
> "mfsprgN gpr".
That may be true but I was asked to get it working. I don't believe
that technically it is correct for m[t,f]sprg to complain that the
register is greater than 3 if you have specified an architecture that
should allow up to 7. Even if so, 'mfsprg %r7,3' will be accepted as
correct but will produce an SPRG number of 275, not 259 as you
previously indicated it should. Same with 'mfsprg3 %r7'.
>>+/* Some dialects (BookE, 405) have 8 SPRG registers instead of
>>+ the normal 4. In addition, mfsprg instructions must
>>+ have sprn5 cleared when using registers 4 through 7. */
>
>
> Is this horrible formatting an artifact of your mailer?
No my editor does that. Easily fixed.
>>+static unsigned long
>>+insert_sprg (unsigned long insn,
>>+ long value,
>>+ int dialect,
>>+ const char **errmsg)
>>+{
>>+ /* This check uses PPC_OPCODE_403 because PPC405 is later defined
>>+ as a synonym. If ever a 405 specific dialect is added this
>>+ check should use that instead. */
>>+ if ((dialect & PPC_OPCODE_BOOKE) || (dialect & PPC_OPCODE_403))
>>+ {
>>+ if (value > 7)
>>+ *errmsg = _("Invalid operand. Must be between 0 and 7.");
>
>
> These error messages should be all lower case, and without a full-stop.
> I suggest simply "invalid sprg number". Then you can restructure the
> function as
>
> if (value > 7
> || (value > 3
> && (dialect & (PPC_OPCODE_BOOKE | PPC_OPCODE_403)) == 0))
> *errmsg = _("invalid sprg number");
>
> insn |= (value & 7) << 16;
> if (value > 3 or maybe 2 && some other condition)
> insn &= bit twiddle;
> return insn;
Easily done once I'm positive what the proper conditions should be.
>>+ 1cc: 7c f3 42 a6 mfsprg r7,3
>>+ 1d0: 7c e7 42 a6 mfsprg7 r7
>>+ 1d4: 7c f7 43 a6 mtsprg 7,r7
>
>
> This isn't the nicest disassembly, a result of mfsprg[3-7] appearing in
> the opcode table before mfsprg.
I know. I briefly thought about implementing an extract_sprg to handle
that but there really isn't a good way to decide how to display it.
>>+ mfsprg %r7, 3
>>+ mfsprg %r7, 7
>>+ mtsprg 7, %r7
>
>
More information about the Binutils
mailing list