[PATCH] Handle mtsprg and mfsprg properly for BookE

Alan Modra amodra@bigpond.net.au
Tue Mar 8 11:20:00 GMT 2005


On Mon, Mar 07, 2005 at 10:23:57AM -0500, Jeff Baker wrote:
> Ping.  Updated for today and confirmed as the correct behaviour.

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..

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".

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

> +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;

> + 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.

> +	mfsprg %r7, 3
> +	mfsprg %r7, 7
> +	mtsprg 7, %r7

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre



More information about the Binutils mailing list