RE: [PATCH] Fix for ldm/stm instructions in H8S

RE: [PATCH] Fix for ldm/stm instructions in H8S

[Asgari J. Jinia]

Hi Nick,
Thank you. I missed this test.
Regards,
Asgari

-----Original Message-----
From: Nick Clifton [mailto:nickc@redhat.com]
Sent: Wednesday, November 26, 2003 6:13 PM
To: Asgari J. Jinia; rsandifo@redhat.com
Cc: binutils@sources.redhat.com
Subject: Re: [PATCH] Fix for ldm/stm instructions in H8S


Hi Asgari, Hi Richard,

: Richard writes:
:
: Hmm, I see the changes you made are the ones below.  Are you sure
: that's right?

Nope - it was just a guess on my part.

: My understanding was that r4-r7 was a valid sequence on the H8SX.  I
: think the tech note only applies to the H8S.


> Asgari writes:
>
> I think t01_mov.s is compiled for H8SX target so register pair
> (er4-er7) is valid as mentioned in H8SX family programming
> manual. So I think those lines from t01_mov.s and t01_mov.exp are
> necessary.

> -    ldm @sp+,(er4-er7)                  ;01306d77
> +    ldm @sp+,(er5-er7)                  ;01206d77

> -    stm (er4-er7),@-sp                  ;01306df4
> +    stm (er5-er7),@-sp                  ;01206df5

I am happy for this part of the patch to be reverted, but if you do
so, you will start generating extra GAS testsuite failures for the
h8300-elf toolchain.

Presumably the correct solution is for the new test in tc-h8300.c to
allow the use of er4-er7 for the H8SX ?  ie something like this:

Index: gas/config/tc-h8300.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-h8300.c,v
retrieving revision 1.43
diff -c -3 -p -r1.43 tc-h8300.c
*** gas/config/tc-h8300.c	25 Nov 2003 23:09:47 -0000	1.43
--- gas/config/tc-h8300.c	26 Nov 2003 12:44:57 -0000
*************** get_operand (char **ptr, struct h8_op *o
*** 598,603 ****
--- 598,604 ----
            && !(low == 2 && (high == 3 || ((high == 4 || high == 5) && SXmode)))
            && !(low == 3 && (high == 4 || high == 5 || high == 6) && SXmode)
            && !(low == 4 && (high == 5 || high == 6))
+           && !(low == 4 && high == 7 && SXmode)
            && !(low == 5 && (high == 6 || high == 7) && SXmode)
            && !(low == 6 && high == 7 && SXmode))
  	as_bad (_("Invalid register list for ldm/stm\n"));

Cheers
        Nick
        

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Nick Clifton]

Hi Asgari,

> Thank you. I missed this test.

OK - I have checked in that patch and reverted the changes to
t01_mov.s and t01_mov.exp.

Cheers
        Nick
        

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Nick Clifton]

Hi Asgari, Hi Richard,

: Richard writes:
:
: Hmm, I see the changes you made are the ones below.  Are you sure
: that's right?

Nope - it was just a guess on my part.

: My understanding was that r4-r7 was a valid sequence on the H8SX.  I
: think the tech note only applies to the H8S.


> Asgari writes:
>
> I think t01_mov.s is compiled for H8SX target so register pair
> (er4-er7) is valid as mentioned in H8SX family programming
> manual. So I think those lines from t01_mov.s and t01_mov.exp are
> necessary.

> -    ldm @sp+,(er4-er7)                  ;01306d77
> +    ldm @sp+,(er5-er7)                  ;01206d77

> -    stm (er4-er7),@-sp                  ;01306df4
> +    stm (er5-er7),@-sp                  ;01206df5

I am happy for this part of the patch to be reverted, but if you do
so, you will start generating extra GAS testsuite failures for the
h8300-elf toolchain.

Presumably the correct solution is for the new test in tc-h8300.c to
allow the use of er4-er7 for the H8SX ?  ie something like this:

Index: gas/config/tc-h8300.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-h8300.c,v
retrieving revision 1.43
diff -c -3 -p -r1.43 tc-h8300.c
*** gas/config/tc-h8300.c	25 Nov 2003 23:09:47 -0000	1.43
--- gas/config/tc-h8300.c	26 Nov 2003 12:44:57 -0000
*************** get_operand (char **ptr, struct h8_op *o
*** 598,603 ****
--- 598,604 ----
            && !(low == 2 && (high == 3 || ((high == 4 || high == 5) && SXmode)))
            && !(low == 3 && (high == 4 || high == 5 || high == 6) && SXmode)
            && !(low == 4 && (high == 5 || high == 6))
+           && !(low == 4 && high == 7 && SXmode)
            && !(low == 5 && (high == 6 || high == 7) && SXmode)
            && !(low == 6 && high == 7 && SXmode))
  	as_bad (_("Invalid register list for ldm/stm\n"));

Cheers
        Nick
        

RE: [PATCH] Fix for ldm/stm instructions in H8S

[Asgari J. Jinia]

Hi Nick,

I think t01_mov.s is compiled for H8SX target so register pair (er4-er7) is valid as mentioned in 
H8SX family programming manual. So I think those lines from t01_mov.s and t01_mov.exp are necessary.

Regards,
Asgari Jinia

-----Original Message-----
From: Richard Sandiford [mailto:rsandifo@redhat.com]
Sent: Wednesday, November 26, 2003 4:27 PM
To: Nick Clifton
Cc: Asgari J. Jinia; kazu@cs.umass.edu; binutils@sources.redhat.com
Subject: Re: [PATCH] Fix for ldm/stm instructions in H8S


Nick Clifton <nickc@redhat.com> writes:
> Hi Asgari,
> > I have modified patch for H8S and H8SX targets. Also I have created
> > patches for multiples.s and h8300.exp in gas/h8300 testsuite. This
> > was tested with binutils-031111 testsuite and found okay.
> 
> Thanks very much - this patch is much better.  There was one small
> omission - the h8300 testcase t01_mov.s had a couple of illegal
> register pairings in it, which lead to an extra 78 unexpected failures
> when the gas testsuite was run for the h8300-elf toolchain.
> 
> I have applied your patch together with a fix to t01_mov.s and
> t01_mov.exp so that there are no new testsuite failures.

Hmm, I see the changes you made are the ones below.  Are you sure
that's right?  My understanding was that r4-r7 was a valid sequence
on the H8SX.  I think the tech note only applies to the H8S.

Richard


Index: t01_mov.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/h8300/t01_mov.s,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- t01_mov.s	5 Jun 2003 18:52:00 -0000	1.1
+++ t01_mov.s	25 Nov 2003 16:13:36 -0000	1.2
@@ -1067,7 +1067,7 @@
     ldm @sp+,(er1-er4)                  ;01306d74
     ldm @sp+,(er2-er5)                  ;01306d75
     ldm @sp+,(er3-er6)                  ;01306d76
-    ldm @sp+,(er4-er7)                  ;01306d77
+    ldm @sp+,(er5-er7)                  ;01206d77
 
     stm (er0-er1),@-sp                  ;01106df0
     stm (er1-er2),@-sp                  ;01106df1
@@ -1088,7 +1088,7 @@
     stm (er1-er4),@-sp                  ;01306df1
     stm (er2-er5),@-sp                  ;01306df2
     stm (er3-er6),@-sp                  ;01306df3
-    stm (er4-er7),@-sp                  ;01306df4
+    stm (er5-er7),@-sp                  ;01206df5
 
     eepmov.b                            ;7b5c598f
 

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Richard Sandiford]

Nick Clifton <nickc@redhat.com> writes:
> Hi Asgari,
> > I have modified patch for H8S and H8SX targets. Also I have created
> > patches for multiples.s and h8300.exp in gas/h8300 testsuite. This
> > was tested with binutils-031111 testsuite and found okay.
> 
> Thanks very much - this patch is much better.  There was one small
> omission - the h8300 testcase t01_mov.s had a couple of illegal
> register pairings in it, which lead to an extra 78 unexpected failures
> when the gas testsuite was run for the h8300-elf toolchain.
> 
> I have applied your patch together with a fix to t01_mov.s and
> t01_mov.exp so that there are no new testsuite failures.

Hmm, I see the changes you made are the ones below.  Are you sure
that's right?  My understanding was that r4-r7 was a valid sequence
on the H8SX.  I think the tech note only applies to the H8S.

Richard


Index: t01_mov.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/h8300/t01_mov.s,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- t01_mov.s	5 Jun 2003 18:52:00 -0000	1.1
+++ t01_mov.s	25 Nov 2003 16:13:36 -0000	1.2
@@ -1067,7 +1067,7 @@
     ldm @sp+,(er1-er4)                  ;01306d74
     ldm @sp+,(er2-er5)                  ;01306d75
     ldm @sp+,(er3-er6)                  ;01306d76
-    ldm @sp+,(er4-er7)                  ;01306d77
+    ldm @sp+,(er5-er7)                  ;01206d77
 
     stm (er0-er1),@-sp                  ;01106df0
     stm (er1-er2),@-sp                  ;01106df1
@@ -1088,7 +1088,7 @@
     stm (er1-er4),@-sp                  ;01306df1
     stm (er2-er5),@-sp                  ;01306df2
     stm (er3-er6),@-sp                  ;01306df3
-    stm (er4-er7),@-sp                  ;01306df4
+    stm (er5-er7),@-sp                  ;01206df5
 
     eepmov.b                            ;7b5c598f
 

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Nick Clifton]

Hi Asgari,

> I have modified patch for H8S and H8SX targets. Also I have created
> patches for multiples.s and h8300.exp in gas/h8300 testsuite. This
> was tested with binutils-031111 testsuite and found okay.

Thanks very much - this patch is much better.  There was one small
omission - the h8300 testcase t01_mov.s had a couple of illegal
register pairings in it, which lead to an extra 78 unexpected failures
when the gas testsuite was run for the h8300-elf toolchain.

I have applied your patch together with a fix to t01_mov.s and
t01_mov.exp so that there are no new testsuite failures.

Cheers
        Nick

gas/ChangeLog
2003-11-25  Asgari Jinia  <asgarij@kpitcummins.com>

	* config/tc-h8300.c (md_assemble): Check operands validity for
	ldm/stm.
	(get_operand): Check register pair's validity as per technical
	note TN-H8*-193A/E from Renesas for H8s and for H8Sx manual.

gas/testsuite/ChangeLog
2003-11-25  Asgari Jinia  <asgarij@kpitcummins.com>

	* gas/h8300/multiples.s : Add instructions for H8s.
	* gas/h8300/h8300.exp (do_h8300s_multiple) : Add more test
	conditions for instructions for H8s.
	* gas/h8300/t01_mov.s: Correct illegal ldm/stm register pairings.
	* gas/h8300/t01_mov.exp: Update expected results.

                

RE: [PATCH] Fix for ldm/stm instructions in H8S

[Asgari J. Jinia]

Hi Nick,

I have modified patch for H8S and H8SX targets. Also I have created patches 
for multiples.s and h8300.exp in gas/h8300 testsuite. This was tested with 
binutils-031111 testsuite and found okay.

Regards,
Asgari Jinia

-------------------- Start of gas patch ---------------------------------
bfd change log:

2003-11-20  Asgari Jinia  <asgarij@kpitcummins.com>

	* config/tc-h8300.c (md_assemble): Check operands validity for
	ldm/stm.
	(get_operand): Check register pair's validity as per technical
	note TN-H8*-193A/E from Renesas for H8s and for H8Sx manual.

--- config/tc-h8300.c.old	2003-11-21 18:04:38.000000000 +0530
+++ config/tc-h8300.c	2003-11-21 18:06:32.000000000 +0530
@@ -622,14 +622,18 @@ get_operand (ptr, op, direction)
       low = src[2] - '0';
       high = src[6] - '0';
 
-      if (high == low)
-	as_bad (_("Invalid register list for ldm/stm\n"));
-
-      if (high < low)
+       /* Check register pair's validity as per tech note TN-H8*-193A/E
+         from Renesas for H8S and H8SX hardware manual.  */
+      if (!(low == 0 && (high == 1 || high == 2 || high == 3))
+          && !(low == 1 && (high == 2 || high == 3 || high == 4) && SXmode)
+          && !(low == 2 && (high == 3 || ((high == 4 || high == 5) && SXmode)))
+          && !(low == 3 && (high == 4 || high == 5 || high == 6) && SXmode)
+          && !(low == 4 && (high == 5 || high == 6))
+          && !(low == 5 && (high == 6 || high == 7) && SXmode)
+          && !(low == 6 && high == 7 && SXmode))
+        {
 	as_bad (_("Invalid register list for ldm/stm\n"));
-
-      if (high - low > 3)
-	as_bad (_("Invalid register list for ldm/stm)\n"));
+        }
 
       /* Even sicker.  We encode two registers into op->reg.  One
 	 for the low register to save, the other for the high
@@ -1965,6 +1969,32 @@ md_assemble (str)
   *op_end = c;
   prev_instruction = instruction;
 
+  /* Now we have operands from instruction.  Let's check them out for
+     ldm and stm.  */
+  if (OP_KIND (instruction->opcode->how) == O_LDM)
+    {
+      /* The first operand must be @er7+, and the second operand must
+          be a register pair.  */
+      if ((operand[0].mode != RSINC)
+           || (operand[0].reg != 7)
+           || ((operand[1].reg & 0x80000000) == 0))
+        {
+          as_bad (_("invalid operand in ldm"));
+        }
+    }
+
+  if (OP_KIND (instruction->opcode->how) == O_STM)
+    {
+      /* The first operand must be a register pair, and the second
+          operand must be @-er7.  */
+      if (((operand[0].reg & 0x80000000) == 0)
+            || (operand[1].mode != RDDEC)
+            || (operand[1].reg != 7))
+        {
+          as_bad (_("invalid operand in stm"));
+        }
+    }
+
   size = SN;
   if (dot)
     {

-------------------- End of gas patch -----------------------------------
--------------------- Start of testsuite patch --------------------------
gas testsuit change log:

2003-11-20  Asgari Jinia  <asgarij@kpitcummins.com>

	* gas/h8300/multiples.s : added instructions for H8s.


--- gas/h8300/multiples.s.old	1999-05-03 12:58:50.000000000 +0530
+++ gas/h8300/multiples.s	2003-11-20 17:27:25.000000000 +0530
@@ -7,4 +7,11 @@ h8300s_multiple:
 	stm.l er0-er1,@-sp
 	stm.l er0-er2,@-sp
 	stm.l er0-er3,@-sp
+        ldm.l @sp+,er2-er3
+        stm.l er2-er3,@-sp
+        ldm.l @sp+,er4-er5
+        ldm.l @sp+,er4-er6
+        stm.l er4-er5,@-sp
+        stm.l er4-er6,@-sp
+
 

2003-11-20  Asgari Jinia  <asgarij@kpitcummins.com>

	* gas/h8300/h8300.exp (do_h8300s_multiple) : added more test conditions for instructions for H8s.


--- gas/h8300/h8300.exp.old	2003-06-24 19:24:50.000000000 +0530
+++ gas/h8300/h8300.exp	2003-11-20 17:31:06.000000000 +0530
@@ -2070,6 +2070,13 @@ proc do_h8300s_multiple {} {
 	    -re " +\[0-9\]+ 000c 01106DF0\[^\n\]*\n"   { set x [expr $x+1] }
 	    -re " +\[0-9\]+ 0010 01206DF0\[^\n\]*\n"   { set x [expr $x+1] }
 	    -re " +\[0-9\]+ 0014 01306DF0\[^\n\]*\n"   { set x [expr $x+1] }
+            -re " +\[0-9\]+ 0018 01106D73\[^\n\]*\n"   { set x [expr $x+1] }
+            -re " +\[0-9\]+ 001c 01106DF2\[^\n\]*\n"   { set x [expr $x+1] }
+            -re " +\[0-9\]+ 0020 01106D75\[^\n\]*\n"   { set x [expr $x+1] }
+            -re " +\[0-9\]+ 0024 01206D76\[^\n\]*\n"   { set x [expr $x+1] }
+            -re " +\[0-9\]+ 0028 01106DF4\[^\n\]*\n"   { set x [expr $x+1] }
+            -re " +\[0-9\]+ 002c 01206DF4\[^\n\]*\n"   { set x [expr $x+1] }
+
 	    eof					{ break }
 	}
     }
@@ -2079,7 +2086,7 @@ proc do_h8300s_multiple {} {
     gas_finish
 
     # Did we find what we were looking for?  If not, flunk it.
-    if [expr $x == 6] then { pass $testname } else { fail $testname }
+    if [expr $x == 12] then { pass $testname } else { fail $testname }
 }
 
 proc do_h8300h_mov32bug {} {


---------------------- End of testsuit patch--------------------------

-----Original Message-----
From: Nick Clifton [mailto:nickc@redhat.com]
Sent: Wednesday, November 12, 2003 12:39 AM
To: Asgari J. Jinia
Cc: kazu@cs.umass.edu; binutils@sources.redhat.com
Subject: Re: [PATCH] Fix for ldm/stm instructions in H8S



Applying this patch breaks lots of tests in the h8300 gas testsuite.
Would it be possible for you to investigate this and see whether the
patch or the testcases need to be fixed ?

Cheers
        Nick
        

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Kazu Hirata]

Hi Asgari,

> The patch finds many invalid instructions in the testsuite in
> './testsuite/gas/h8300/t01_mov.s'. So because of that it generates
> many FAILS.
> 
> However, H8SX supports many register pairs in ldm/stm instructions
> which are not supported in H8S. For this reason, I have removed test
> to check register pair validity in this patch. I will create
> separate patch for register pair's test for ldm/stm instructions as
> it will involve changes in the testsuite as well.

I guess the test for register range in your previous patch is good.
It just needs a check for which target you're assembling for.  That's
a good piece of code.  Yon don't have to completely remove that test!?
You may have to adjust the testsuite.

Kazu Hirata

RE: [PATCH] Fix for ldm/stm instructions in H8S

[Asgari J. Jinia]

Hello Nick,

The patch finds many invalid instructions in the testsuite in './testsuite/gas/h8300/t01_mov.s'. So because of that it generates many FAILS.


However, H8SX supports many register pairs in ldm/stm instructions which are not supported in H8S. For this reason, I have removed test to check register pair validity in this patch. I will create separate patch for register pair's test for ldm/stm instructions as it will involve changes in the testsuite as well.

Please find updated patch as below which tests for operands in ldm/stm instructions. I have tested it with binutils-031111 testsuite and found ok.

Regards,
Asgari Jinia

--------------- start of patch ------------------------------------

2003-11-14  Asgari Jinia  <asgarij@kpitcummins.com>

	* config/tc-h8300.c (md_assemble): Check operands validity for
	ldm/stm.



--- ./config/tc-h8300.c.old	2003-10-17 15:53:34.000000000 +0530
+++ ./config/tc-h8300.c	2003-11-14 15:38:32.000000000 +0530
@@ -1965,6 +1965,32 @@ md_assemble (str)
   *op_end = c;
   prev_instruction = instruction;
 
+  /* Now we have operands from instuction.  Let's check them out for
+     ldm and stm.  */
+  if (OP_KIND (instruction->opcode->how) == O_LDM)
+    {
+      /* The first operand must be @er7+, and the second operand must
+          be a register pair.  */
+      if ((operand[0].mode != RSINC)
+           || (operand[0].reg != 7)
+           || ((operand[1].reg & 0x80000000) == 0))
+        {
+          as_bad (_("invalid operand in ldm"));
+        }
+    }
+
+  if (OP_KIND (instruction->opcode->how) == O_STM)
+    {
+      /* The first operand must be a register pair, and the second
+          operand must be @-er7.  */
+      if (((operand[0].reg & 0x80000000) == 0)
+            || (operand[1].mode != RDDEC)
+            || (operand[1].reg != 7))
+        {
+          as_bad (_("invalid operand in stm"));
+        }
+    }
+
   size = SN;
   if (dot)
     {
------------------------ End of patch --------------------------------------------------

-----Original Message-----
From: Nick Clifton [mailto:nickc@redhat.com]
Sent: Wednesday, November 12, 2003 12:39 AM
To: Asgari J. Jinia
Cc: kazu@cs.umass.edu; binutils@sources.redhat.com
Subject: Re: [PATCH] Fix for ldm/stm instructions in H8S


Applying this patch breaks lots of tests in the h8300 gas testsuite.
Would it be possible for you to investigate this and see whether the
patch or the testcases need to be fixed ?

Cheers
        Nick
        

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Nick Clifton]

Hi Asgari,

> Thanks for your suggestion. The test for register validity suggested
> by you is simpler so we can go ahead with that. Please find attached
> a test program which was used for testing and results were found
> ok. Please find below patch.

Applying this patch breaks lots of tests in the h8300 gas testsuite.
Would it be possible for you to investigate this and see whether the
patch or the testcases need to be fixed ?

Cheers
        Nick
        
> 2003-11-05  Asgari Jinia  <asgarij@kpitcummins.com>
> 
> 	* config/tc-h8300.c (md_assemble): Check operands validity for
> 	ldm/stm.
> 	(get_operand): Check register pair's validity as per technical
> 	note TN-H8*-193A/E from Renesas.

        

RE: [PATCH] Fix for ldm/stm instructions in H8S

[Asgari J. Jinia]

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

Hi Kazu,

Thanks for your suggestion. The test for register validity suggested by you is simpler so we can go ahead with that. Please find attached a test program which was used for testing and results were found ok. Please find below patch.

Regards,
Asgari Jinia

------------------ Start Of Patch ---------------------------
2003-11-07  Asgari Jinia  <asgarij@kpitcummins.com>

	* config/tc-h8300.c (md_assemble): Check operands validity for
	ldm/stm.
	(get_operand): Check register pair's validity as per technical
	note TN-H8*-193A/E from Renesas.


--- tc-h8300.old.c	Fri Nov  7 17:59:46 2003
+++ tc-h8300.c	Fri Nov  7 18:17:20 2003
@@ -622,14 +622,14 @@ get_operand (ptr, op, direction)
       low = src[2] - '0';
       high = src[6] - '0';
 
-      if (high == low)
-	as_bad (_("Invalid register list for ldm/stm\n"));
-
-      if (high < low)
+       /* Check register pair's validity as per tech note TN-H8*-193A/E
+         from Renesas.  */
+      if (!(low == 0 && (high == 1 || high == 2 || high == 3))
+          && !(low == 2 && high == 3)
+          && !(low == 4 && (high == 5 || high == 6)))
+        {
 	as_bad (_("Invalid register list for ldm/stm\n"));
-
-      if (high - low > 3)
-	as_bad (_("Invalid register list for ldm/stm)\n"));
+        }
 
       /* Even sicker.  We encode two registers into op->reg.  One
 	 for the low register to save, the other for the high
@@ -1965,6 +1965,32 @@ md_assemble (str)
   *op_end = c;
   prev_instruction = instruction;
 
+  /* Now we have operands from instuction.  Let's check them out for
+     ldm and stm.  */
+  if (OP_KIND (instruction->opcode->how) == O_LDM)
+    {
+      /* The first operand must be @er7+, and the second operand must
+          be a register pair.  */
+      if ((operand[0].mode != RSINC)
+           || (operand[0].reg != 7)
+           || ((operand[1].reg & 0x80000000) == 0))
+        {
+          as_bad (_("invalid operand in ldm"));
+        }
+    }
+
+  if (OP_KIND (instruction->opcode->how) == O_STM)
+    {
+      /* The first operand must be a register pair, and the second
+          operand must be @-er7.  */
+      if (((operand[0].reg & 0x80000000) == 0)
+            || (operand[1].mode != RDDEC)
+            || (operand[1].reg != 7))
+        {
+          as_bad (_("invalid operand in stm"));
+        }
+    }
+
   size = SN;
   if (dot)
     {
------------------ End Of Patch ---------------------------

-----Original Message-----
From: Kazu Hirata [mailto:kazu@cs.umass.edu]
Sent: Thursday, November 06, 2003 4:24 AM
To: Asgari J. Jinia
Cc: binutils@sources.redhat.com
Subject: Re: [PATCH] Fix for ldm/stm instructions in H8S


Hi Asgari,

> 2003-11-05 Asgari Jinia <asgarij@kpitcummins.com>
> 	* config/tc-h8300.c (md_assemble) : Check operands validity for ldm/stm.
> 	(get_operand) : Check register pair's validity as per tech note TN-H8*-193A/E from Renesas.

What about a simpler test that checks all possibilities of low and
high (see attached)?  Someone reading the code does not have to
decipher it.

By the way, I fixed minor things like formatting.  Next time you
contribute something, please follow the GNU Coding Standards,
available at:

  http://www.gnu.org/prep/standards_toc.html

I assume you actually fed some invalid code to get these error
messages.  The patch looks good.  I leave the approval up to other
people.

Otherwise

Kazu Hirata

2003-11-05  Asgari Jinia  <asgarij@kpitcummins.com>

	* config/tc-h8300.c (md_assemble): Check operands validity for
	ldm/stm.
	(get_operand): Check register pair's validity as per technical
	note TN-H8*-193A/E from Renesas.

Index: tc-h8300.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-h8300.c,v
retrieving revision 1.37
diff -c -r1.37 tc-h8300.c
*** tc-h8300.c	17 Oct 2003 10:23:33 -0000	1.37
--- tc-h8300.c	5 Nov 2003 22:36:46 -0000
***************
*** 622,635 ****
        low = src[2] - '0';
        high = src[6] - '0';
  
!       if (high == low)
! 	as_bad (_("Invalid register list for ldm/stm\n"));
! 
!       if (high < low)
! 	as_bad (_("Invalid register list for ldm/stm\n"));
! 
!       if (high - low > 3)
! 	as_bad (_("Invalid register list for ldm/stm)\n"));
  
        /* Even sicker.  We encode two registers into op->reg.  One
  	 for the low register to save, the other for the high
--- 622,635 ----
        low = src[2] - '0';
        high = src[6] - '0';
  
!       /* Check register pair's validity as per tech note TN-H8*-193A/E
! 	 from Renesas.  */
!       if (!(low == 0 && (high == 1 || high == 2 || high == 3))
! 	  && !(low == 2 && high == 3)
! 	  && !(low == 4 && (high == 5 || high == 6)))
! 	{
! 	  as_bad (_("Invalid register list for ldm/stm\n"));
! 	}
  
        /* Even sicker.  We encode two registers into op->reg.  One
  	 for the low register to save, the other for the high
***************
*** 1965,1970 ****
--- 1965,1995 ----
    *op_end = c;
    prev_instruction = instruction;
  
+    /* Now we have operands from instuction.  Let's check them out for
+       ldm and stm.  */
+    if (OP_KIND (instruction->opcode->how) == O_LDM)
+      {
+        /* The first operand must be @er7+, and the second operand must
+ 	  be a register pair.  */
+        if ((operand[0].mode != RSINC)
+ 	   || (operand[0].reg != 7)
+ 	   || ((operand[1].reg & 0x80000000) == 0))
+          {
+            as_bad (_("invalid operand in ldm"));
+          }
+      }
+  
+    if (OP_KIND (instruction->opcode->how) == O_STM)
+      {
+        /* The first operand must be a register pair, and the second
+ 	  operand must be @-er7.  */
+        if (((operand[0].reg & 0x80000000) == 0)
+ 	   || (operand[1].mode != RDDEC) || (operand[1].reg != 7))
+          {
+            as_bad (_("invalid operand in stm"));
+          }
+      }
+  
    size = SN;
    if (dot)
      {

[-- Attachment #2: main.s --]
[-- Type: application/octet-stream, Size: 1049 bytes --]

	.h8300s
	.section .text
	.global	_test_ldm

_test_ldm:

	ldm.l @sp+, er0-er1 ; Legal
	ldm.l @sp+, er2-er3 ; Legal
	ldm.l @sp+, er4-er5 ; Legal
	ldm.l @sp+, er6-er7 ; Illegal

	ldm.l @sp+, er0-er2 ; Legal
	ldm.l @sp+, er4-er6 ; Legal

	ldm.l @sp+, er0-er3 ; Legal
	ldm.l @sp+, er4-er7 ; Illegal
	ldm.l er0-er1, @-sp ; Illegal
	ldm.l er2-er3, @-sp ; Illegal
	ldm.l er4-er5, @-sp ; Illegal
	ldm.l er6-er7, @-sp ; Illegal

	ldm.l er0-er2, @-sp ; Illegal
	ldm.l er4-er6, @-sp ; Illegal

	ldm.l er0-er3, @-sp ; Illegal
	ldm.l er4-er7, @-sp ; Illegal

	stm.l er0-er1, @-sp ; Legal	
	stm.l er2-er3, @-sp ; Legal
	stm.l er4-er5, @-sp ; Legal
	stm.l er6-er7, @-sp ; Illigal	
	stm.l er0-er2, @-sp ; Legal
	stm.l er4-er6, @-sp ; Legal

	stm.l er0-er3, @-sp ; Legal
	stm.l er4-er7, @-sp ; Illigal

	stm.l @sp+, er0-er1 ; Illegal
	stm.l @sp+, er2-er3 ; Illegal
	stm.l @sp+, er4-er5 ; Illegal
	stm.l @sp+, er6-er7 ; Illegal
	stm.l @sp+, er0-er2 ; Illegal
	stm.l @sp+, er4-er6 ; Illegal

	stm.l @sp+, er0-er3 ; Illegal
	stm.l @sp+, er4-er7 ; Illegal
	
	rts

	.end

Re: [PATCH] Fix for ldm/stm instructions in H8S

[Kazu Hirata]

Hi Asgari,

> 2003-11-05 Asgari Jinia <asgarij@kpitcummins.com>
> 	* config/tc-h8300.c (md_assemble) : Check operands validity for ldm/stm.
> 	(get_operand) : Check register pair's validity as per tech note TN-H8*-193A/E from Renesas.

What about a simpler test that checks all possibilities of low and
high (see attached)?  Someone reading the code does not have to
decipher it.

By the way, I fixed minor things like formatting.  Next time you
contribute something, please follow the GNU Coding Standards,
available at:

  http://www.gnu.org/prep/standards_toc.html

I assume you actually fed some invalid code to get these error
messages.  The patch looks good.  I leave the approval up to other
people.

Otherwise

Kazu Hirata

2003-11-05  Asgari Jinia  <asgarij@kpitcummins.com>

	* config/tc-h8300.c (md_assemble): Check operands validity for
	ldm/stm.
	(get_operand): Check register pair's validity as per technical
	note TN-H8*-193A/E from Renesas.

Index: tc-h8300.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-h8300.c,v
retrieving revision 1.37
diff -c -r1.37 tc-h8300.c
*** tc-h8300.c	17 Oct 2003 10:23:33 -0000	1.37
--- tc-h8300.c	5 Nov 2003 22:36:46 -0000
***************
*** 622,635 ****
        low = src[2] - '0';
        high = src[6] - '0';
  
!       if (high == low)
! 	as_bad (_("Invalid register list for ldm/stm\n"));
! 
!       if (high < low)
! 	as_bad (_("Invalid register list for ldm/stm\n"));
! 
!       if (high - low > 3)
! 	as_bad (_("Invalid register list for ldm/stm)\n"));
  
        /* Even sicker.  We encode two registers into op->reg.  One
  	 for the low register to save, the other for the high
--- 622,635 ----
        low = src[2] - '0';
        high = src[6] - '0';
  
!       /* Check register pair's validity as per tech note TN-H8*-193A/E
! 	 from Renesas.  */
!       if (!(low == 0 && (high == 1 || high == 2 || high == 3))
! 	  && !(low == 2 && high == 3)
! 	  && !(low == 4 && (high == 5 || high == 6)))
! 	{
! 	  as_bad (_("Invalid register list for ldm/stm\n"));
! 	}
  
        /* Even sicker.  We encode two registers into op->reg.  One
  	 for the low register to save, the other for the high
***************
*** 1965,1970 ****
--- 1965,1995 ----
    *op_end = c;
    prev_instruction = instruction;
  
+    /* Now we have operands from instuction.  Let's check them out for
+       ldm and stm.  */
+    if (OP_KIND (instruction->opcode->how) == O_LDM)
+      {
+        /* The first operand must be @er7+, and the second operand must
+ 	  be a register pair.  */
+        if ((operand[0].mode != RSINC)
+ 	   || (operand[0].reg != 7)
+ 	   || ((operand[1].reg & 0x80000000) == 0))
+          {
+            as_bad (_("invalid operand in ldm"));
+          }
+      }
+  
+    if (OP_KIND (instruction->opcode->how) == O_STM)
+      {
+        /* The first operand must be a register pair, and the second
+ 	  operand must be @-er7.  */
+        if (((operand[0].reg & 0x80000000) == 0)
+ 	   || (operand[1].mode != RDDEC) || (operand[1].reg != 7))
+          {
+            as_bad (_("invalid operand in stm"));
+          }
+      }
+  
    size = SN;
    if (dot)
      {

[PATCH] Fix for ldm/stm instructions in H8S

[Asgari J. Jinia]

Hi,
Please find below a patch for H8 assembler which fixes few bugs for ldm/stm instructions for H8S.
Description:

1. For LDM the first operand cannot be register list and for STM.L second operand cannot be register list. 
   At present the assembler accepts this.
2. In LDM first operand must be @er7+ and in STM second operand must be @-er7. At present it accepts other 
   than er7.
3. According to technical note ( TN-H8*-193A/E ) by Renesas available at 
   http://www.eu.renesas.com/documents/mpumcu/tech/h8193e.pdf , er7 cannot be used in register list with 
   LDM/STM instructions. It checks register pair as per this technical note.

Regards,
Asgari Jinia

-------------------- Start Of Patch -------------------------------
gas/Changelog-
2003-11-05 Asgari Jinia <asgarij@kpitcummins.com>
	* config/tc-h8300.c (md_assemble) : Check operands validity for ldm/stm.
	(get_operand) : Check register pair's validity as per tech note TN-H8*-193A/E from Renesas.


--- gas/config/tc-h8300.old.c	Fri Oct 17 15:53:34 2003
+++ gas/config/tc-h8300.c	Wed Nov  5 16:44:31 2003
@@ -622,14 +622,13 @@ get_operand (ptr, op, direction)
       low = src[2] - '0';
       high = src[6] - '0';
 
-      if (high == low)
-	as_bad (_("Invalid register list for ldm/stm\n"));
-
-      if (high < low)
+      /*check register pair's validity as per tech note TN-H8*-193A/E
+        from Renesas*/
+      if ((high <= low) || (high - low > 3) || (low % 2) || (low > 4)
+           || ((high - low == 2) && (low % 4)) || ((high - low == 3) && low))
+        {
 	as_bad (_("Invalid register list for ldm/stm\n"));
-
-      if (high - low > 3)
-	as_bad (_("Invalid register list for ldm/stm)\n"));
+        }
 
       /* Even sicker.  We encode two registers into op->reg.  One
 	 for the low register to save, the other for the high
@@ -1965,6 +1964,25 @@ md_assemble (str)
   *op_end = c;
   prev_instruction = instruction;
 
+  /* now we have operands from instuction. Lets check out them for ldm and stm*/
+  if (OP_KIND (instruction->opcode->how) == O_LDM)
+    {
+      /*first operand must be @er7+ and second operand must be register pair*/
+      if ((operand[0].mode != RSINC) || (operand[0].reg != 7) || ((operand[1].reg & 0x80000000) == 0))
+        {
+          as_bad (_("invalid operand in ldm"));
+        }
+    }
+
+  if (OP_KIND (instruction->opcode->how) == O_STM)
+    {
+      /*first operand must be register pair and second operand must be @-er7 */
+      if (((operand[0].reg & 0x80000000) == 0) || (operand[1].mode != RDDEC) || (operand[1].reg != 7))
+        {
+          as_bad (_("invalid operand in stm"));
+        }
+    }
+
   size = SN;
   if (dot)
     {
-------------------- End Of Patch -------------------------------