RE: [patch] gas/config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

RE: [patch] gas/config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

[Kazu Hirata]

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

Hi Jeff,

> Can you please install a test for this bug into the testsuite?
> 
> I believe Nick C. already approved the tc-h8300.c change.

Would you like the attached patch? This certainly avoids the coincidence.

Thanks,

Kazu


[-- Attachment #2: ChangeLog-ldmac --]
[-- Type: text/plain, Size: 160 bytes --]

2000-06-27  Kazu Hirata  <kazu@hxi.com>

	* gas/h8300/h8300.exp (do_h8300s_mac): Change the registers used
	in ldmac and stmac.
	* gas/h8300/macs.s: Likewise.


[-- Attachment #3: ldmac.patch --]
[-- Type: text/x-diff, Size: 1639 bytes --]

Index: gas/testsuite/gas/h8300/h8300.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/h8300/h8300.exp,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 h8300.exp
--- h8300.exp	1999/05/03 07:28:49	1.1.1.1
+++ h8300.exp	2000/06/27 13:14:22
@@ -2037,11 +2037,11 @@
     while 1 {
 	expect {
 	    -re " +\[0-9\]+ 0000 01A0\[^\n\]*\n"   { set x [expr $x+1] }
-	    -re " +\[0-9\]+ 0002 0320\[^\n\]*\n"   { set x [expr $x+1] }
-	    -re " +\[0-9\]+ 0004 0331\[^\n\]*\n"   { set x [expr $x+1] }
-	    -re " +\[0-9\]+ 0006 01606D01\[^\n\]*\n"   { set x [expr $x+1] }
-	    -re " +\[0-9\]+ 000a 0220\[^\n\]*\n"   { set x [expr $x+1] }
-	    -re " +\[0-9\]+ 000c 0231\[^\n\]*\n"   { set x [expr $x+1] }
+	    -re " +\[0-9\]+ 0002 0324\[^\n\]*\n"   { set x [expr $x+1] }
+	    -re " +\[0-9\]+ 0004 0335\[^\n\]*\n"   { set x [expr $x+1] }
+	    -re " +\[0-9\]+ 0006 01606D45\[^\n\]*\n"   { set x [expr $x+1] }
+	    -re " +\[0-9\]+ 000a 0224\[^\n\]*\n"   { set x [expr $x+1] }
+	    -re " +\[0-9\]+ 000c 0235\[^\n\]*\n"   { set x [expr $x+1] }
 	    eof					{ break }
 	}
     }
Index: gas/testsuite/gas/h8300/macs.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/h8300/macs.s,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 macs.s
--- macs.s	1999/05/03 07:28:49	1.1.1.1
+++ macs.s	2000/06/27 13:14:22
@@ -2,10 +2,8 @@
 	.text
 h8300s_mac:
 	clrmac
-	ldmac er0,mach
-	ldmac er1,macl
-	mac @er0+,@er1+
-	stmac mach,er0
-	stmac macl,er1
-	
-
+	ldmac er4,mach
+	ldmac er5,macl
+	mac @er4+,@er5+
+	stmac mach,er4
+	stmac macl,er5

Re: [patch] gas/config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

[Nick Clifton]

Hi Kazu,

: 2000-06-27  Kazu Hirata  <kazu@hxi.com>=0A=
: 
: 	* gas/h8300/h8300.exp (do_h8300s_mac): Change the registers used
: 	in ldmac and stmac.
: 	* gas/h8300/macs.s: Likewise.

Approved and applied.

Cheers
	Nick

Re: [patch] gas/config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

[Jeffrey A Law]

  In message <200006242323.QAA05447@cygnus.com>you write:
  > Hi,
  > 
  > Attached is a patch to fix a bug in h8300-hms-as. The original gas
  > does not assemble the first two lines of the following function
  > correctly. The last two lines are OK.
  > 
  > /* h8300-hms-gcc -ms -Wall -O2 -fomit-frame-pointer */
  > void
  > test ()
  > {
  >   __asm__ __volatile__ ("ldmac\ter4,mach");
  >   __asm__ __volatile__ ("ldmac\ter5,macl");
  >   __asm__ __volatile__ ("stmac\tmach,er4");
  >   __asm__ __volatile__ ("stmac\tmacl,er5");
  > }
  > 
  > Without the patch,
  > 
  > 00000000 <_test>:
  >    0:	03 64       	03 64             ldmac	er4,mach <- incorrect
  >    2:	03 75       	03 75             ldmac	er5,mach <- incorrect
  >    4:	02 24       	02 24             stmac	mach,er4
  >    6:	02 35       	02 35             stmac	macl,er5
  >    8:	54 70       	54 70             rts	
  > 
  > With the patch, 
  > 
  > 00000000 <_test>:
  >    0:	03 24       	03 24             ldmac	er4,mach <- correct
  >    2:	03 35       	03 35             ldmac	er5,macl <- correct
  >    4:	02 24       	02 24             stmac	mach,er4
  >    6:	02 35       	02 35             stmac	macl,er5
  >    8:	54 70       	54 70             rts	
  > 
  > The problem is that the variable 'd' in 'build_bytes ()' is always 0,
  > meaning the first operand, when processing MACREG. However, as you can
  > see from the assembly code, when we want to know which half of the mac
  > register (mach or macl) is used, we have to look at either the first
  > or second operand depending on ldmac or stmac.
  > 
  > By the way, the existing testsuite (gas/testsuite/gas/h8300/h8300.exp
  > and macs.s) is correct. The reason this bug got away with a failure is
  > a coincident that you would never expect. If you change the 32-bit
  > registers (er0 and er1) used in macs.s to some other 32-bit registers
  > (say, er4 and er5 just like the above example) and change h8300.exp
  > accordingly, the original gas would fail.
Can you please install a test for this bug into the testsuite?

I believe Nick C. already approved the tc-h8300.c change.
Thanks,
jeff

Re: [patch] gas/config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

[Nick Clifton]

Hi Kazu,

: 2000-06-24  Kazu Hirata  <kazu@hxi.com>
: 
: 	* config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

Accepted and applied - thanks.

Cheers
	Nick

[patch] gas/config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

[Kazu Hirata]

Hi,

Attached is a patch to fix a bug in h8300-hms-as. The original gas
does not assemble the first two lines of the following function
correctly. The last two lines are OK.

/* h8300-hms-gcc -ms -Wall -O2 -fomit-frame-pointer */
void
test ()
{
  __asm__ __volatile__ ("ldmac\ter4,mach");
  __asm__ __volatile__ ("ldmac\ter5,macl");
  __asm__ __volatile__ ("stmac\tmach,er4");
  __asm__ __volatile__ ("stmac\tmacl,er5");
}

Without the patch,

00000000 <_test>:
   0:	03 64       	03 64             ldmac	er4,mach <- incorrect
   2:	03 75       	03 75             ldmac	er5,mach <- incorrect
   4:	02 24       	02 24             stmac	mach,er4
   6:	02 35       	02 35             stmac	macl,er5
   8:	54 70       	54 70             rts	

With the patch, 

00000000 <_test>:
   0:	03 24       	03 24             ldmac	er4,mach <- correct
   2:	03 35       	03 35             ldmac	er5,macl <- correct
   4:	02 24       	02 24             stmac	mach,er4
   6:	02 35       	02 35             stmac	macl,er5
   8:	54 70       	54 70             rts	

The problem is that the variable 'd' in 'build_bytes ()' is always 0,
meaning the first operand, when processing MACREG. However, as you can
see from the assembly code, when we want to know which half of the mac
register (mach or macl) is used, we have to look at either the first
or second operand depending on ldmac or stmac.

By the way, the existing testsuite (gas/testsuite/gas/h8300/h8300.exp
and macs.s) is correct. The reason this bug got away with a failure is
a coincident that you would never expect. If you change the 32-bit
registers (er0 and er1) used in macs.s to some other 32-bit registers
(say, er4 and er5 just like the above example) and change h8300.exp
accordingly, the original gas would fail.

Thanks,

Kazu

===File ~/gnu/binutils/ChangeLog-tc-h8300===================
2000-06-24  Kazu Hirata  <kazu@hxi.com>

	* config/tc-h8300.c (build_bytes): Assemble ldmac correctly.

============================================================

===File ~/gnu/binutils/tc-h8300.patch=======================
Index: gas/config/tc-h8300.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-h8300.c,v
retrieving revision 1.6
diff -u -r1.6 tc-h8300.c
--- tc-h8300.c	2000/06/23 05:44:30	1.6
+++ tc-h8300.c	2000/06/24 23:03:03
@@ -1026,7 +1026,12 @@
 
 	  if (c & MACREG)
 	    {
-	      nib = 2 + operand[d].reg;
+	      if (operand[0].mode == MACREG)
+		/* stmac has mac[hl] as the first operand.  */
+		nib = 2 + operand[0].reg;
+	      else
+		/* ldmac has mac[hl] as the second operand.  */
+		nib = 2 + operand[1].reg;
 	    }
 	}
       nibble_count++;
============================================================