Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[H . J . Lu]

The new gas generates different opcodes on Linux/mips. The change was
introduced by the HAVE_32BIT_GPRS change. In include/opcode/mips.h

/* Test for membership in an ISA including chip specific ISAs.
   INSN is pointer to an element of the opcode table; ISA is the
   specified ISA to test against; and CPU is the CPU specific ISA
   to test, or zero if no CPU specific ISA test is desired.
   The gp32 arg is set when you need to force 32-bit register usage on
   a machine with 64-bit registers; see the documentation under -mgp32
   in the MIPS gas docs.  */

#define OPCODE_IS_MEMBER(insn, isa, cpu, gp32)                          \

I don't understand why the change was made. Here is a patch which
restores the old opcodes. But I am not sure if it is 100% correct.
Any comments?


H.J.
----
2001-08-07  H.J. Lu  <hjl@gnu.org>

	* config/tc-mips.c (HAVE_FORCEED_32BIT_GPRS): New.
	(macro_build): Pass HAVE_FORCEED_32BIT_GPRS to OPCODE_IS_MEMBER
	instead of HAVE_32BIT_GPRS.
	(mips_ip): Likewise.

--- gas/config/tc-mips.c.gp32	Tue Aug  7 12:46:48 2001
+++ gas/config/tc-mips.c	Tue Aug  7 12:48:11 2001
@@ -266,6 +266,9 @@ static int mips_32bit_abi = 0;
     || mips_32bit_abi                       \
     || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
 
+#define HAVE_FORCEED_32BIT_GPRS	            \
+   (mips_gp32 || mips_32bit_abi)
+
 #define HAVE_32BIT_FPRS                     \
    (mips_fp32                               \
     || mips_32bit_abi                       \
@@ -2618,7 +2621,7 @@ macro_build (place, counter, ep, name, f
       if (strcmp (fmt, insn.insn_mo->args) == 0
 	  && insn.insn_mo->pinfo != INSN_MACRO
 	  && OPCODE_IS_MEMBER (insn.insn_mo, mips_opts.isa, mips_arch,
-			       HAVE_32BIT_GPRS)
+			       HAVE_FORCEED_32BIT_GPRS)
 	  && (mips_arch != CPU_R4650 || (insn.insn_mo->pinfo & FP_D) == 0))
 	break;
 
@@ -7154,7 +7157,8 @@ mips_ip (str, ip)
 
       assert (strcmp (insn->name, str) == 0);
 
-      if (OPCODE_IS_MEMBER (insn, mips_opts.isa, mips_arch, HAVE_32BIT_GPRS))
+      if (OPCODE_IS_MEMBER (insn, mips_opts.isa, mips_arch,
+			    HAVE_FORCEED_32BIT_GPRS))
 	ok = true;
       else
 	ok = false;

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Richard Sandiford]

"H . J . Lu" <hjl@lucon.org> writes:

> The new gas generates different opcodes on Linux/mips. The change was
> introduced by the HAVE_32BIT_GPRS change. In include/opcode/mips.h
> 
> /* Test for membership in an ISA including chip specific ISAs.
>    INSN is pointer to an element of the opcode table; ISA is the
>    specified ISA to test against; and CPU is the CPU specific ISA
>    to test, or zero if no CPU specific ISA test is desired.
>    The gp32 arg is set when you need to force 32-bit register usage on
>    a machine with 64-bit registers; see the documentation under -mgp32
>    in the MIPS gas docs.  */
> 
> #define OPCODE_IS_MEMBER(insn, isa, cpu, gp32)                          \
> 
> I don't understand why the change was made. Here is a patch which
> restores the old opcodes. But I am not sure if it is 100% correct.
> Any comments?

I'm not sure why you want to make this change.  Because you want "addi" to be
used instead of "ori" for the move instruction?  It should be OK to change
mips-opc.c so that "addi" is used for -mgp32 if so.

There was discussion after I sent the patch of making the mtune option
decide which instruction is used for moves.  That sounded like a great idea,
so I held off making a mips-opc.c change myself.

Your patch seems to assume that "-mips2" and "-mips2 -mgp32" should give
different results, which doesn't make much sense to me.

Richard

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[H . J . Lu]

On Tue, Aug 07, 2001 at 09:14:36PM +0100, Richard Sandiford wrote:
> "H . J . Lu" <hjl@lucon.org> writes:
> 
> > The new gas generates different opcodes on Linux/mips. The change was
> > introduced by the HAVE_32BIT_GPRS change. In include/opcode/mips.h
> > 
> > /* Test for membership in an ISA including chip specific ISAs.
> >    INSN is pointer to an element of the opcode table; ISA is the
> >    specified ISA to test against; and CPU is the CPU specific ISA
> >    to test, or zero if no CPU specific ISA test is desired.
> >    The gp32 arg is set when you need to force 32-bit register usage on
> >    a machine with 64-bit registers; see the documentation under -mgp32
> >    in the MIPS gas docs.  */
> > 
> > #define OPCODE_IS_MEMBER(insn, isa, cpu, gp32)                          \
> > 
> > I don't understand why the change was made. Here is a patch which
> > restores the old opcodes. But I am not sure if it is 100% correct.
> > Any comments?
> 
> I'm not sure why you want to make this change.  Because you want "addi" to be
> used instead of "ori" for the move instruction?  It should be OK to change

It is "addu" vs. "or" for "move $1, $2".

> mips-opc.c so that "addi" is used for -mgp32 if so.
> 
> There was discussion after I sent the patch of making the mtune option
> decide which instruction is used for moves.  That sounded like a great idea,
> so I held off making a mips-opc.c change myself.
> 
> Your patch seems to assume that "-mips2" and "-mips2 -mgp32" should give
> different results, which doesn't make much sense to me.

That was the old behavior, which I don't know if it makes senses or not. That
is why I am asking. Maybe we should do

#define HAVE_FORCEED_32BIT_GPRS			\
   ((mips_gp32 || mips_32bit_abi)		\
    && ISA_HAS_64BIT_REGS (mips_opts.isa))

I cannot tell any difference between "addu" vs. "or" for "move". But I
don't think we should change it for no good reason.


H.J.

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Thiemo Seufer]

H . J . Lu wrote:
[snip]
>    The gp32 arg is set when you need to force 32-bit register usage on
>    a machine with 64-bit registers; see the documentation under -mgp32
>    in the MIPS gas docs.  */

This comment is possibly misleading. There are three ways to restrict
the assembler to 32bit GPRs: Explicitly by the programmer (-mgp32),
implicitly by the ABI (-mabi=32) and implicitly by the ISA (-mips{1,2,32}
or -march=FOO), where only the latter one can change over an assembly
run. So we need to check only two states: Programmer+ABI and ISA.
The current code checks for three which may get inconsistent.

[snip]
> +#define HAVE_FORCEED_32BIT_GPRS	            \
                     ^
Consistent typo. :-)

> +   (mips_gp32 || mips_32bit_abi)
> +

Why differentiate between forced and non-forced 32bit GPRS?
The assembler should handle both identically.

AFAICS mips_32bit_abi should imply mips_gp32 (and mips_fp32).
Then the check for mips_32bit_abi in HAVE_32BIT_?PRS can go
away and everyone is happy.


Thiemo

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[H . J . Lu]

On Tue, Aug 07, 2001 at 10:43:16PM +0200, Thiemo Seufer wrote:
> H . J . Lu wrote:
> [snip]
> >    The gp32 arg is set when you need to force 32-bit register usage on
> >    a machine with 64-bit registers; see the documentation under -mgp32
> >    in the MIPS gas docs.  */
> 
> This comment is possibly misleading. There are three ways to restrict
> the assembler to 32bit GPRs: Explicitly by the programmer (-mgp32),
> implicitly by the ABI (-mabi=32) and implicitly by the ISA (-mips{1,2,32}
> or -march=FOO), where only the latter one can change over an assembly
> run. So we need to check only two states: Programmer+ABI and ISA.
> The current code checks for three which may get inconsistent.

There are

/* 32-bit code running on a ISA3+ CPU.  */
#define INSN_GP32                 0x00100000

#define OPCODE_IS_MEMBER(insn, isa, cpu, gp32)                          \
    ((((insn)->membership & isa) != 0                                   \
      && ((insn)->membership & INSN_GP32 ? gp32 : 1)                    \

#define G6      INSN_GP32

{"move", "d,s", 0x00000025, 0xfc1f07ff, WR_d|RD_s, I1|G6   },/* or */

"move" is the only one affected by this. I am enclosing a new patch here.

> > +#define HAVE_FORCEED_32BIT_GPRS	            \
>                      ^
> Consistent typo. :-)

Oops.

> 
> > +   (mips_gp32 || mips_32bit_abi)
> > +
> 
> Why differentiate between forced and non-forced 32bit GPRS?
> The assembler should handle both identically.

I don't know.


H.J.
--- gas/config/tc-mips.c.gp32	Tue Aug  7 12:46:48 2001
+++ gas/config/tc-mips.c	Tue Aug  7 14:05:41 2001
@@ -266,6 +266,10 @@ static int mips_32bit_abi = 0;
     || mips_32bit_abi                       \
     || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
 
+#define HAVE_FORCED_32BIT_GPRS	            \
+   ((mips_gp32 || mips_32bit_abi)	    \
+    && ISA_HAS_64BIT_REGS (mips_opts.isa))
+
 #define HAVE_32BIT_FPRS                     \
    (mips_fp32                               \
     || mips_32bit_abi                       \
@@ -2618,7 +2622,7 @@ macro_build (place, counter, ep, name, f
       if (strcmp (fmt, insn.insn_mo->args) == 0
 	  && insn.insn_mo->pinfo != INSN_MACRO
 	  && OPCODE_IS_MEMBER (insn.insn_mo, mips_opts.isa, mips_arch,
-			       HAVE_32BIT_GPRS)
+			       HAVE_FORCED_32BIT_GPRS)
 	  && (mips_arch != CPU_R4650 || (insn.insn_mo->pinfo & FP_D) == 0))
 	break;
 
@@ -7154,7 +7158,8 @@ mips_ip (str, ip)
 
       assert (strcmp (insn->name, str) == 0);
 
-      if (OPCODE_IS_MEMBER (insn, mips_opts.isa, mips_arch, HAVE_32BIT_GPRS))
+      if (OPCODE_IS_MEMBER (insn, mips_opts.isa, mips_arch,
+			    HAVE_FORCED_32BIT_GPRS))
 	ok = true;
       else
 	ok = false;

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[cgd]

ica2_ts@csv.ica.uni-stuttgart.de ("Thiemo Seufer") writes:
> Why differentiate between forced and non-forced 32bit GPRS?
> The assembler should handle both identically.

So, I think the answer to this is that by some chain of logic you
might end up with 'non-canonical 32-bit values' (i.e. 32-bit values
but where bits 63:32 != bit 31) in registers, if you're in
forced-32-bit mode.

In that case, you would need to use 'or' or 'daddu' to avoid
unimplemented behaviour, but you might not be able to use daddu
because the processor is configured to disallow it.

I'm not sure what I think of that answer for -mgp32, and don't know
enough about -mabi=32 to speak about what should be done there.


To go back to the original patch message:

	http://sources.redhat.com/ml/binutils/1999-12/msg00123.html

It and the messages that preceeded it:

	http://sources.redhat.com/ml/binutils/1999-10/msg00133.html
	http://sources.redhat.com/ml/binutils/1999-10/msg00135.html

shed some light on the issue.

In fact, -mgp32 seems to be intended for when you're running w/o
64-bit extensions enabled, on mips3 and later parts.  (e.g. MIPS64
in user mode with SR:PX set to 0.)

In that case, you _can't_ get non-canonical values in the registers;
any operations that you would attempt to produce those values would
instead trap as reserved or illegal.

I don't know how abi=32 code is intended to be run.  if it simply
defines calling conventions, and not processor mode, then you need to
use 'or' rather than 'addu'.  If it defines processor mode as well,
then addu would be sufficient.


In any event, my personal preference is to nuke the gp32 arg to
OPCODE_IS_MEMBER, and assemble "move" as a macro, and use -mtune to
schedule preference of 'or' vs. 'add'.

I think i'd code the selection like:

	if ((mips_isa & INSN_ISA3)
	    && (forced 32-bit with gp32 or abi=32)) {

		emit 'or' for sanity.

	} else {

		switch (tune) {

		/* processor tuning here!!! */

		default:
			move32_insn = "addu";
			move64_insn = "daddu";
		}

		if (mips_isa & INSN_ISA3)
			emit move64_insn;
		else
			emit move32_insn;
	}

or use better (existing) checks for 64-bit ISA rather than the
(mips_ISA & INSN_ISA3) above.  8-)


cgd

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Thiemo Seufer]

cgd@broadcom.com wrote:
> ica2_ts@csv.ica.uni-stuttgart.de ("Thiemo Seufer") writes:
> > Why differentiate between forced and non-forced 32bit GPRS?
> > The assembler should handle both identically.
> 
> So, I think the answer to this is that by some chain of logic you
> might end up with 'non-canonical 32-bit values' (i.e. 32-bit values
> but where bits 63:32 != bit 31) in registers, if you're in
> forced-32-bit mode.

Such code is broken by definition. Every 32bit value has to be
properly sign-extended on a 64bit MIPS CPU. Otherwise it's
behaviour is undefined, at least WRT the MIPS IV spec.

[snip]
> To go back to the original patch message:
> 
> 	http://sources.redhat.com/ml/binutils/1999-12/msg00123.html

Ah, that's the patch which also introduced the setting of mips_64
in -mgp{32,64}. This is wrong because mips_64 denotes the use of
-mabi=64, not the use of 64bit insns.

> It and the messages that preceeded it:
> 
> 	http://sources.redhat.com/ml/binutils/1999-10/msg00133.html
> 	http://sources.redhat.com/ml/binutils/1999-10/msg00135.html
> 
> shed some light on the issue.

Especially about the 'or' vs. 'addu' thing. I already assumed that
'or' is the wrong choice, and Ian says there so, too.

[snip]
> I don't know how abi=32 code is intended to be run.  if it simply
> defines calling conventions, and not processor mode,

For our purpose it defines register widths and calling conventions,
but not the processor mode.

> then you need to
> use 'or' rather than 'addu'.  If it defines processor mode as well,
> then addu would be sufficient.

No, addu is ok for any ISA conforming code due to the requirement
of proper sign extension.

> In any event, my personal preference is to nuke the gp32 arg to
> OPCODE_IS_MEMBER, and assemble "move" as a macro, and use -mtune to
> schedule preference of 'or' vs. 'add'.

Even without the need for 'or' this would be the IMHO cleaner
solution.


Thiemo

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Thiemo Seufer]

H . J . Lu wrote:
[snip]
> > >    The gp32 arg is set when you need to force 32-bit register usage on
> > >    a machine with 64-bit registers; see the documentation under -mgp32
> > >    in the MIPS gas docs.  */
> > 
> > This comment is possibly misleading. There are three ways to restrict
> > the assembler to 32bit GPRs: Explicitly by the programmer (-mgp32),
> > implicitly by the ABI (-mabi=32) and implicitly by the ISA (-mips{1,2,32}
> > or -march=FOO), where only the latter one can change over an assembly
> > run. So we need to check only two states: Programmer+ABI and ISA.
> > The current code checks for three which may get inconsistent.
> 
> There are
> 
> /* 32-bit code running on a ISA3+ CPU.  */
> #define INSN_GP32                 0x00100000
> 
> #define OPCODE_IS_MEMBER(insn, isa, cpu, gp32)                          \
>     ((((insn)->membership & isa) != 0                                   \
>       && ((insn)->membership & INSN_GP32 ? gp32 : 1)                    \
> 
> #define G6      INSN_GP32
> 
> {"move", "d,s", 0x00000025, 0xfc1f07ff, WR_d|RD_s, I1|G6   },/* or */
> 
> "move" is the only one affected by this. I am enclosing a new patch here.

Oh, that's all ok, but I advocated against your HAVE_FORCED_32BIT_GPRS.

[snip]
> > > +   (mips_gp32 || mips_32bit_abi)
> > > +
> > 
> > Why differentiate between forced and non-forced 32bit GPRS?
> > The assembler should handle both identically.
> 
> I don't know.

After reading the archived email mentioned by Chris,
http://sources.redhat.com/ml/binutils/1999-12/msg00123.html
and especially
http://sources.redhat.com/ml/binutils/1999-10/msg00132.html
I assume the use of 'or' instead of the usual 'addu' was not a
thoughtful decision but a 'low cost fix' to get working code.

So I vote to use 'addu' instead of 'or'.


Thiemo

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Richard Sandiford]

cgd@broadcom.com writes:

> In fact, -mgp32 seems to be intended for when you're running w/o
> 64-bit extensions enabled, on mips3 and later parts.  (e.g. MIPS64
> in user mode with SR:PX set to 0.)

FWIW, that's how I read it too.  It seemed that there wasn't really a
preference for "or" over "addu" if -mgp32 was given.  The original -mgp32
change to the opcode table seemed to be more about avoiding "daddu".

> In that case, you _can't_ get non-canonical values in the registers;
> any operations that you would attempt to produce those values would
> instead trap as reserved or illegal.
>
> I don't know how abi=32 code is intended to be run.  if it simply
> defines calling conventions, and not processor mode, then you need to
> use 'or' rather than 'addu'.  If it defines processor mode as well,
> then addu would be sufficient.

My understanding was that abi=32 should imply processor mode as well.  If
there isn't agreement on that, let me know!  Then I'll go back to using
forced-gpr-32 mode only if -mgp32 is explicitly given.

As it stands, many macros will use 32-bit instructions for address
calculations if abi=32.  These operations would be just as unsafe as an
"addu" move instruction if registers aren't in canonical form.

I don't think that there's really any reason why "move" should be treated
specially here.  It seems to be a choice between register size and ABI
either being linked or being kept seperate.  If people would prefer them
kept separate (the historical behaviour) then I'm willing to change it back.

Richard

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Thiemo Seufer]

Richard Sandiford wrote:
[snip]
> > I don't know how abi=32 code is intended to be run.  if it simply
> > defines calling conventions, and not processor mode, then you need to
> > use 'or' rather than 'addu'.  If it defines processor mode as well,
> > then addu would be sufficient.
> 
> My understanding was that abi=32 should imply processor mode as well.

It can't, since 64bit modes were introduced after 32bit ABI.

> If
> there isn't agreement on that, let me know!  Then I'll go back to using
> forced-gpr-32 mode only if -mgp32 is explicitly given.
> 
> As it stands, many macros will use 32-bit instructions for address
> calculations if abi=32.  These operations would be just as unsafe as an
> "addu" move instruction if registers aren't in canonical form.

This is why the ISA requires sign extension of 32bit addresses.

> I don't think that there's really any reason why "move" should be treated
> specially here.  It seems to be a choice between register size and ABI
> either being linked or being kept seperate.  If people would prefer them
> kept separate (the historical behaviour) then I'm willing to change it back.

IMHO it would be weird to request ABI-conforming code without
ABI-conforming register sizes.


Thiemo

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Richard Sandiford]

Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> writes:

> > In any event, my personal preference is to nuke the gp32 arg to
> > OPCODE_IS_MEMBER, and assemble "move" as a macro, and use -mtune to
> > schedule preference of 'or' vs. 'add'.
> 
> Even without the need for 'or' this would be the IMHO cleaner
> solution.

Same here.  Is the patch below OK?  It uses "addu" instead of "or" with
-mgp32 and -mabi=32.  Can change it to use "or" if the concensus is that
that's safer.

Tested on mips-elf mips64-elf and mipsel-elf.

[include/ChangeLog]

	* opcode/mips.h (INSN_GP32): Remove.
	(OPCODE_IS_MEMBER): Remove gp32 parameter.
	(M_MOVE): New macro identifier.

[opcodes/ChangeLog]

	* mips-dis.c (print_insn_mips): Remove OPCODE_IS_MEMBER's gp32
	argument.
	* mips-opc.c (G6): Undefine.
	(mips_builtin_opcodes): Remove gp32 entry for "move".  Add macro
	as the first "move" alternative.

[gas/ChangeLog]

	* config/tc-mips.c (move_register): New function.
	(macro_build): Remove OPCODE_IS_MEMBER's gp32 argument.
	(mips_ip): Likewise.
	(macro2): Use move_register rather than macro_build for moves.
	(mips16_macro): Likewise.
	(macro): Likewise.  Handle M_MOVE.

[gas/testsuite/ChangeLog]

	* gas/mips/mips-gp32-fp32.d: Expect addu rather than or for moves.
	* gas/mips/mips-gp32-fp64.d: Likewise.
	* gas/mips/mips-abi32.d: Likewise.
	* gas/mips/mips-gp32-fp32-pic.d: Likewise.
	* gas/mips/mips-gp32-fp64-pic.d: Likewise.
	* gas/mips/mips-abi32-pic.d: Likewise.
	

Index: include/opcode/mips.h
===================================================================
RCS file: /cvs/src/src/include/opcode/mips.h,v
retrieving revision 1.16
diff -u -p -d -r1.16 mips.h
--- include/opcode/mips.h	2001/08/01 11:33:44	1.16
+++ include/opcode/mips.h	2001/08/08 11:48:04
@@ -326,8 +326,6 @@ struct mips_opcode
 #define INSN_4100                 0x00040000
 /* Toshiba R3900 instruction.  */
 #define INSN_3900                 0x00080000
-/* 32-bit code running on a ISA3+ CPU.  */
-#define INSN_GP32                 0x00100000
 
 /* MIPS ISA defines, use instead of hardcoding ISA level.  */
 
@@ -369,15 +367,10 @@ struct mips_opcode
 /* Test for membership in an ISA including chip specific ISAs.
    INSN is pointer to an element of the opcode table; ISA is the
    specified ISA to test against; and CPU is the CPU specific ISA
-   to test, or zero if no CPU specific ISA test is desired.
-   The gp32 arg is set when you need to force 32-bit register usage on
-   a machine with 64-bit registers; see the documentation under -mgp32
-   in the MIPS gas docs.  */
+   to test, or zero if no CPU specific ISA test is desired.  */
 
-#define OPCODE_IS_MEMBER(insn, isa, cpu, gp32)				\
-    ((((insn)->membership & isa) != 0                           	\
-      && ((insn)->membership & INSN_GP32 ? gp32 : 1)			\
-     )									\
+#define OPCODE_IS_MEMBER(insn, isa, cpu)				\
+    (((insn)->membership & isa) != 0					\
      || (cpu == CPU_R4650 && ((insn)->membership & INSN_4650) != 0)	\
      || (cpu == CPU_R4010 && ((insn)->membership & INSN_4010) != 0)	\
      || ((cpu == CPU_VR4100 || cpu == CPU_R4111)			\
@@ -508,6 +501,7 @@ enum
   M_LWR_A,
   M_LWR_AB,
   M_LWU_AB,
+  M_MOVE,
   M_MUL,
   M_MUL_I,
   M_MULO,
Index: opcodes/mips-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/mips-dis.c,v
retrieving revision 1.17
diff -u -p -d -r1.17 mips-dis.c
--- opcodes/mips-dis.c	2001/08/07 12:36:13	1.17
+++ opcodes/mips-dis.c	2001/08/08 11:48:04
@@ -469,7 +469,7 @@ print_insn_mips (memaddr, word, info)
 	    {
 	      register const char *d;
 
-	      if (! OPCODE_IS_MEMBER (op, mips_isa, target_processor, 0))
+	      if (! OPCODE_IS_MEMBER (op, mips_isa, target_processor))
 		continue;
 
 	      (*info->fprintf_func) (info->stream, "%s", op->name);
Index: opcodes/mips-opc.c
===================================================================
RCS file: /cvs/src/src/opcodes/mips-opc.c,v
retrieving revision 1.23
diff -u -p -d -r1.23 mips-opc.c
--- opcodes/mips-opc.c	2001/07/21 20:46:51	1.23
+++ opcodes/mips-opc.c	2001/08/08 11:48:04
@@ -96,8 +96,6 @@ Software Foundation, 59 Temple Place - S
 #define G3 (I4             \
             )
 
-#define G6      INSN_GP32
-
 /* The order of overloaded instructions matters.  Label arguments and
    register arguments look the same. Instructions that can have either
    for arguments must apear in the correct order in this table for the
@@ -121,7 +119,7 @@ const struct mips_opcode mips_builtin_op
 {"li",      "t,j",      0x24000000, 0xffe00000, WR_t,			I1	}, /* addiu */
 {"li",	    "t,i",	0x34000000, 0xffe00000, WR_t,			I1	}, /* ori */
 {"li",      "t,I",	0,    (int) M_LI,	INSN_MACRO,		I1	},
-{"move",    "d,s",	0x00000025, 0xfc1f07ff,	WR_d|RD_s,		I1|G6	},/* or */
+{"move",    "d,s",	0,    (int) M_MOVE,	INSN_MACRO,		I1	},
 {"move",    "d,s",	0x0000002d, 0xfc1f07ff, WR_d|RD_s,		I3	},/* daddu */
 {"move",    "d,s",	0x00000021, 0xfc1f07ff, WR_d|RD_s,		I1	},/* addu */
 {"move",    "d,s",	0x00000025, 0xfc1f07ff,	WR_d|RD_s,		I1	},/* or */
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.60
diff -u -p -d -r1.60 tc-mips.c
--- gas/config/tc-mips.c	2001/08/07 13:22:46	1.60
+++ gas/config/tc-mips.c	2001/08/08 11:48:06
@@ -685,6 +685,7 @@ static void check_absolute_expr PARAMS (
 					 expressionS *));
 static void load_register PARAMS ((int *, int, expressionS *, int));
 static void load_address PARAMS ((int *counter, int reg, expressionS *ep));
+static void move_register PARAMS ((int *, int, int));
 static void macro PARAMS ((struct mips_cl_insn * ip));
 static void mips16_macro PARAMS ((struct mips_cl_insn * ip));
 #ifdef LOSING_COMPILER
@@ -2533,8 +2534,7 @@ macro_build (place, counter, ep, name, f
     {
       if (strcmp (fmt, insn.insn_mo->args) == 0
 	  && insn.insn_mo->pinfo != INSN_MACRO
-	  && OPCODE_IS_MEMBER (insn.insn_mo, mips_opts.isa, mips_arch,
-			       HAVE_32BIT_GPRS)
+	  && OPCODE_IS_MEMBER (insn.insn_mo, mips_opts.isa, mips_arch)
 	  && (mips_arch != CPU_R4650 || (insn.insn_mo->pinfo & FP_D) == 0))
 	break;
 
@@ -3430,6 +3430,19 @@ load_address (counter, reg, ep)
     abort ();
 }
 
+/* Move the contents of register SOURCE into register DEST.  */
+
+static void
+move_register (counter, dest, source)
+     int *counter;
+     int dest;
+     int source;
+{
+  macro_build ((char *) NULL, counter, (expressionS *) NULL,
+	       HAVE_32BIT_GPRS ? "addu" : "daddu",
+	       "d,v,t", dest, source, 0);
+}
+
 /*
  *			Build macros
  *   This routine implements the seemingly endless macro or synthesized
@@ -3503,7 +3516,7 @@ macro (ip)
       if (dreg == sreg)
 	macro_build ((char *) NULL, &icnt, NULL, "nop", "", 0);
       else
-	macro_build ((char *) NULL, &icnt, NULL, "move", "d,s", dreg, sreg, 0);
+	move_register (&icnt, dreg, sreg);
       macro_build ((char *) NULL, &icnt, NULL,
 		   dbl ? "dsub" : "sub", "d,v,t", dreg, 0, sreg);
 
@@ -4056,10 +4069,9 @@ macro (ip)
       if (imm_expr.X_op == O_constant && imm_expr.X_add_number == 1)
 	{
 	  if (strcmp (s2, "mflo") == 0)
-	    macro_build ((char *) NULL, &icnt, NULL, "move", "d,s", dreg,
-			 sreg);
+	    move_register (&icnt, dreg, sreg);
 	  else
-	    macro_build ((char *) NULL, &icnt, NULL, "move", "d,s", dreg, 0);
+	    move_register (&icnt, dreg, 0);
 	  return;
 	}
       if (imm_expr.X_op == O_constant
@@ -4072,7 +4084,7 @@ macro (ip)
 			   "d,w", dreg, sreg);
 	    }
 	  else
-	    macro_build ((char *) NULL, &icnt, NULL, "move", "d,s", dreg, 0);
+	    move_register (&icnt, dreg, 0);
 	  return;
 	}
 
@@ -5189,8 +5201,7 @@ macro (ip)
 	      if (lreg <= 31)
 		{
 		  if (offset_expr.X_op == O_absent)
-		    macro_build ((char *) NULL, &icnt, NULL, "move", "d,s",
-				 lreg, 0);
+		    move_register (&icnt, lreg, 0);
 		  else
 		    {
 		      assert (offset_expr.X_op == O_constant);
@@ -5808,6 +5819,10 @@ macro (ip)
 		   ip->insn_opcode);
       return;
 
+    case M_MOVE:
+      move_register (&icnt, dreg, sreg);
+      return;
+
 #ifdef LOSING_COMPILER
     default:
       /* Try and see if this is a new itbl instruction.
@@ -6039,7 +6054,7 @@ macro2 (ip)
 	{
 	  as_warn (_("Instruction %s: result is always false"),
 		   ip->insn_mo->name);
-	  macro_build ((char *) NULL, &icnt, NULL, "move", "d,s", dreg, 0);
+	  move_register (&icnt, dreg, 0);
 	  return;
 	}
       if (imm_expr.X_op == O_constant
@@ -6778,8 +6793,7 @@ mips16_macro (ip)
       expr1.X_add_number = 0;
       macro_build ((char *) NULL, &icnt, &expr1, "slti", "x,8",  yreg);
       if (xreg != yreg)
-	macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
-		     "move", "y,X", xreg, yreg);
+	move_register (&icnt, xreg, yreg);
       expr1.X_add_number = 2;
       macro_build ((char *) NULL, &icnt, &expr1, "bteqz", "p");
       macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
@@ -6948,7 +6962,7 @@ mips_ip (str, ip)
 
       assert (strcmp (insn->name, str) == 0);
 
-      if (OPCODE_IS_MEMBER (insn, mips_opts.isa, mips_arch, HAVE_32BIT_GPRS))
+      if (OPCODE_IS_MEMBER (insn, mips_opts.isa, mips_arch))
 	ok = true;
       else
 	ok = false;
Index: gas/testsuite/gas/mips/mips-gp32-fp32.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips-gp32-fp32.d,v
retrieving revision 1.2
diff -u -p -d -r1.2 mips-gp32-fp32.d
--- gas/testsuite/gas/mips/mips-gp32-fp32.d	2001/08/05 00:53:28	1.2
+++ gas/testsuite/gas/mips/mips-gp32-fp32.d	2001/08/08 11:48:06
@@ -60,7 +60,7 @@ Disassembly of section .text:
   c8:	a8240000 	swl	a0,0\(at\)
   cc:	b8240003 	swr	a0,3\(at\)
   d0:	3c043ff0 	lui	a0,0x3ff0
-  d4:	00002825 	move	a1,zero
+  d4:	00002821 	move	a1,zero
   d8:	3c010000 	lui	at,0x0
   dc:	8c240000 	lw	a0,0\(at\)
   e0:	8c250004 	lw	a1,4\(at\)
@@ -72,7 +72,7 @@ Disassembly of section .text:
   f8:	2c840001 	sltiu	a0,a0,1
   fc:	24a40064 	addiu	a0,a1,100
  100:	0004202b 	sltu	a0,zero,a0
- 104:	00a02025 	move	a0,a1
+ 104:	00a02021 	move	a0,a1
 
 0+0108 <[^>]*>:
 	...
Index: gas/testsuite/gas/mips/mips-gp32-fp32-pic.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips-gp32-fp32-pic.d,v
retrieving revision 1.2
diff -u -p -d -r1.2 mips-gp32-fp32-pic.d
--- gas/testsuite/gas/mips/mips-gp32-fp32-pic.d	2001/08/05 00:53:28	1.2
+++ gas/testsuite/gas/mips/mips-gp32-fp32-pic.d	2001/08/08 11:48:06
@@ -98,7 +98,7 @@ Disassembly of section .text:
  160:	a8240000 	swl	a0,0\(at\)
  164:	b8240003 	swr	a0,3\(at\)
  168:	3c043ff0 	lui	a0,0x3ff0
- 16c:	00002825 	move	a1,zero
+ 16c:	00002821 	move	a1,zero
  170:	8f810000 	lw	at,0\(gp\)
  174:	8c240000 	lw	a0,0\(at\)
  178:	8c250004 	lw	a1,4\(at\)
@@ -111,7 +111,7 @@ Disassembly of section .text:
  194:	2c840001 	sltiu	a0,a0,1
  198:	24a40064 	addiu	a0,a1,100
  19c:	0004202b 	sltu	a0,zero,a0
- 1a0:	00a02025 	move	a0,a1
+ 1a0:	00a02021 	move	a0,a1
 
 0+01a4 <[^>]*>:
 	...
Index: gas/testsuite/gas/mips/mips-gp32-fp64.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips-gp32-fp64.d,v
retrieving revision 1.2
diff -u -p -d -r1.2 mips-gp32-fp64.d
--- gas/testsuite/gas/mips/mips-gp32-fp64.d	2001/08/05 00:53:28	1.2
+++ gas/testsuite/gas/mips/mips-gp32-fp64.d	2001/08/08 11:48:06
@@ -60,7 +60,7 @@ Disassembly of section .text:
   c8:	a8240000 	swl	a0,0\(at\)
   cc:	b8240003 	swr	a0,3\(at\)
   d0:	3c043ff0 	lui	a0,0x3ff0
-  d4:	00002825 	move	a1,zero
+  d4:	00002821 	move	a1,zero
   d8:	3c010000 	lui	at,0x0
   dc:	8c240000 	lw	a0,0\(at\)
   e0:	8c250004 	lw	a1,4\(at\)
@@ -70,7 +70,7 @@ Disassembly of section .text:
   f0:	2c840001 	sltiu	a0,a0,1
   f4:	24a40064 	addiu	a0,a1,100
   f8:	0004202b 	sltu	a0,zero,a0
-  fc:	00a02025 	move	a0,a1
+  fc:	00a02021 	move	a0,a1
  100:	46231040 	add.d	\$f1,\$f2,\$f3
 
 0+0104 <[^>]*>:
Index: gas/testsuite/gas/mips/mips-gp32-fp64-pic.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips-gp32-fp64-pic.d,v
retrieving revision 1.2
diff -u -p -d -r1.2 mips-gp32-fp64-pic.d
--- gas/testsuite/gas/mips/mips-gp32-fp64-pic.d	2001/08/05 00:53:28	1.2
+++ gas/testsuite/gas/mips/mips-gp32-fp64-pic.d	2001/08/08 11:48:06
@@ -98,7 +98,7 @@ Disassembly of section .text:
  160:	a8240000 	swl	a0,0\(at\)
  164:	b8240003 	swr	a0,3\(at\)
  168:	3c043ff0 	lui	a0,0x3ff0
- 16c:	00002825 	move	a1,zero
+ 16c:	00002821 	move	a1,zero
  170:	8f810000 	lw	at,0\(gp\)
  174:	8c240000 	lw	a0,0\(at\)
  178:	8c250004 	lw	a1,4\(at\)
@@ -110,7 +110,7 @@ Disassembly of section .text:
  190:	2c840001 	sltiu	a0,a0,1
  194:	24a40064 	addiu	a0,a1,100
  198:	0004202b 	sltu	a0,zero,a0
- 19c:	00a02025 	move	a0,a1
+ 19c:	00a02021 	move	a0,a1
  1a0:	46231040 	add.d	\$f1,\$f2,\$f3
 
 0+01a4 <[^>]*>:
Index: gas/testsuite/gas/mips/mips-abi32.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips-abi32.d,v
retrieving revision 1.2
diff -u -p -d -r1.2 mips-abi32.d
--- gas/testsuite/gas/mips/mips-abi32.d	2001/08/05 00:53:28	1.2
+++ gas/testsuite/gas/mips/mips-abi32.d	2001/08/08 11:48:06
@@ -60,7 +60,7 @@ Disassembly of section .text:
   c8:	a8240000 	swl	a0,0\(at\)
   cc:	b8240003 	swr	a0,3\(at\)
   d0:	3c043ff0 	lui	a0,0x3ff0
-  d4:	00002825 	move	a1,zero
+  d4:	00002821 	move	a1,zero
   d8:	3c010000 	lui	at,0x0
   dc:	8c240000 	lw	a0,0\(at\)
   e0:	8c250004 	lw	a1,4\(at\)
@@ -72,7 +72,7 @@ Disassembly of section .text:
   f8:	2c840001 	sltiu	a0,a0,1
   fc:	24a40064 	addiu	a0,a1,100
  100:	0004202b 	sltu	a0,zero,a0
- 104:	00a02025 	move	a0,a1
+ 104:	00a02021 	move	a0,a1
 
 0+0108 <[^>]*>:
 	...
Index: gas/testsuite/gas/mips/mips-abi32-pic.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips-abi32-pic.d,v
retrieving revision 1.2
diff -u -p -d -r1.2 mips-abi32-pic.d
--- gas/testsuite/gas/mips/mips-abi32-pic.d	2001/08/05 00:53:28	1.2
+++ gas/testsuite/gas/mips/mips-abi32-pic.d	2001/08/08 11:48:06
@@ -98,7 +98,7 @@ Disassembly of section .text:
  160:	a8240000 	swl	a0,0\(at\)
  164:	b8240003 	swr	a0,3\(at\)
  168:	3c043ff0 	lui	a0,0x3ff0
- 16c:	00002825 	move	a1,zero
+ 16c:	00002821 	move	a1,zero
  170:	8f810000 	lw	at,0\(gp\)
  174:	8c240000 	lw	a0,0\(at\)
  178:	8c250004 	lw	a1,4\(at\)
@@ -111,7 +111,7 @@ Disassembly of section .text:
  194:	2c840001 	sltiu	a0,a0,1
  198:	24a40064 	addiu	a0,a1,100
  19c:	0004202b 	sltu	a0,zero,a0
- 1a0:	00a02025 	move	a0,a1
+ 1a0:	00a02021 	move	a0,a1
 
 0+01a4 <[^>]*>:
 	...

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Eric Christopher]

> Same here.  Is the patch below OK?  It uses "addu" instead of "or" with
> -mgp32 and -mabi=32.  Can change it to use "or" if the concensus is that
> that's safer.
> 

I usually prefer or, but at this point I think it is an aesthetic
preference :)

Anyhow, this looks ok to me.  Did you test gcc as well?  mips-elf should
give you enough combinations (mipsI/II,III/IV)...

-eric

> Tested on mips-elf mips64-elf and mipsel-elf.
> 
> [include/ChangeLog]
> 
> 	* opcode/mips.h (INSN_GP32): Remove.
> 	(OPCODE_IS_MEMBER): Remove gp32 parameter.
> 	(M_MOVE): New macro identifier.
> 
> [opcodes/ChangeLog]
> 
> 	* mips-dis.c (print_insn_mips): Remove OPCODE_IS_MEMBER's gp32
> 	argument.
> 	* mips-opc.c (G6): Undefine.
> 	(mips_builtin_opcodes): Remove gp32 entry for "move".  Add macro
> 	as the first "move" alternative.
> 
> [gas/ChangeLog]
> 
> 	* config/tc-mips.c (move_register): New function.
> 	(macro_build): Remove OPCODE_IS_MEMBER's gp32 argument.
> 	(mips_ip): Likewise.
> 	(macro2): Use move_register rather than macro_build for moves.
> 	(mips16_macro): Likewise.
> 	(macro): Likewise.  Handle M_MOVE.
> 
> [gas/testsuite/ChangeLog]
> 
> 	* gas/mips/mips-gp32-fp32.d: Expect addu rather than or for moves.
> 	* gas/mips/mips-gp32-fp64.d: Likewise.
> 	* gas/mips/mips-abi32.d: Likewise.
> 	* gas/mips/mips-gp32-fp32-pic.d: Likewise.
> 	* gas/mips/mips-gp32-fp64-pic.d: Likewise.
> 	* gas/mips/mips-abi32-pic.d: Likewise.
> 


-- 
Look out behind you!

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Thiemo Seufer]

Eric Christopher wrote:
> 
> > Same here.  Is the patch below OK?  It uses "addu" instead of "or" with
> > -mgp32 and -mabi=32.  Can change it to use "or" if the concensus is that
> > that's safer.
> > 
> 
> I usually prefer or, but at this point I think it is an aesthetic
> preference :)

It's not only aesthetics, some CPU's can issue 2 'addu' at the same
time but only one 'or'.

> Anyhow, this looks ok to me.

Same here. In review the INSN_GP32 looked weird amongst the
processor specific defines.


Thiemo

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Eric Christopher]

> It's not only aesthetics, some CPU's can issue 2 'addu' at the same
> time but only one 'or'.
> 

Newfangled dual issue processors :)

If 'addu' is more efficient then we should do that by default :)

> > Anyhow, this looks ok to me.
> 
> Same here. In review the INSN_GP32 looked weird amongst the
> processor specific defines.
> 

Yeah... minor though :)

-eric


-- 
Look out behind you!

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Richard Sandiford]

Eric Christopher <echristo@redhat.com> writes:

> Anyhow, this looks ok to me.  Did you test gcc as well?  mips-elf should
> give you enough combinations (mipsI/II,III/IV)...

Always worth checking.  I just did a mips-elf test run on the simulator
(mips1/O32, mips3/O32, mips3/N64, mips4/O32 and mips4/N64), didn't show
up any regressions.  OK to install?

> > Tested on mips-elf mips64-elf and mipsel-elf.
> > 
> > [include/ChangeLog]
> > 
> > 	* opcode/mips.h (INSN_GP32): Remove.
> > 	(OPCODE_IS_MEMBER): Remove gp32 parameter.
> > 	(M_MOVE): New macro identifier.
> > 
> > [opcodes/ChangeLog]
> > 
> > 	* mips-dis.c (print_insn_mips): Remove OPCODE_IS_MEMBER's gp32
> > 	argument.
> > 	* mips-opc.c (G6): Undefine.
> > 	(mips_builtin_opcodes): Remove gp32 entry for "move".  Add macro
> > 	as the first "move" alternative.
> > 
> > [gas/ChangeLog]
> > 
> > 	* config/tc-mips.c (move_register): New function.
> > 	(macro_build): Remove OPCODE_IS_MEMBER's gp32 argument.
> > 	(mips_ip): Likewise.
> > 	(macro2): Use move_register rather than macro_build for moves.
> > 	(mips16_macro): Likewise.
> > 	(macro): Likewise.  Handle M_MOVE.
> > 
> > [gas/testsuite/ChangeLog]
> > 
> > 	* gas/mips/mips-gp32-fp32.d: Expect addu rather than or for moves.
> > 	* gas/mips/mips-gp32-fp64.d: Likewise.
> > 	* gas/mips/mips-abi32.d: Likewise.
> > 	* gas/mips/mips-gp32-fp32-pic.d: Likewise.
> > 	* gas/mips/mips-gp32-fp64-pic.d: Likewise.
> > 	* gas/mips/mips-abi32-pic.d: Likewise.

Re: Why pass HAVE_32BIT_GPRS to OPCODE_IS_MEMBER?

[Eric Christopher]

On 10 Aug 2001 17:08:46 +0100, Richard Sandiford wrote:
> Eric Christopher <echristo@redhat.com> writes:
> 
> > Anyhow, this looks ok to me.  Did you test gcc as well?  mips-elf should
> > give you enough combinations (mipsI/II,III/IV)...
> 
> Always worth checking.  I just did a mips-elf test run on the simulator
> (mips1/O32, mips3/O32, mips3/N64, mips4/O32 and mips4/N64), didn't show
> up any regressions.  OK to install?
> 

Yep.  Thanks.

-eric

-- 
Look out behind you!