[PATCH] : SH Assembler generates incorrect opcode for PCMP instructions

[PATCH] : SH Assembler generates incorrect opcode for PCMP instructions

[Arati Dikey]

Hi !

The SH assembler generates incorrect opcodes for the parallel PCMP
instruction.
Basically, its last nibble is a copy of the previous instruction's
second nibble
when working in Big Endian format.

For example,
test.s
	movs.w @-R5, A0
      PCMP X0,Y0

generates opcodes
	f5 70
	f8 00 84 05

 instead of
	f8 00 84 00

The following patch corrects this. I have also verified that it does not
cause any
side effect on other DSP instructions.

Regards,
Arati Dikey


Changelog 
	2002-01-17  Arati Dikey <aratid@kpit.com>

	tc-sh.c (assemble_ppi): Do not add reg_n value for PCMP
instructions



--- tc-sh.c.orig	Thu Dec  6 11:34:18 2001
+++ tc-sh.c	      Thu Jan 17 16:41:28 2002
@@ -1769,7 +1769,10 @@ assemble_ppi (op_end, opcode)
 	  if (field_b)
 	    as_bad (_("multiple parallel processing specifications"));
 	  field_b = ((opcode->nibbles[2] << 12) + (opcode->nibbles[3] <<
8)
-		     + (reg_x << 6) + (reg_y << 4) + reg_n);
+		     + (reg_x << 6) + (reg_y << 4) );
+
+	  if (strcmp (opcode->name, "pcmp") != 0)
+	  	 field_b += reg_n;
 	  break;
 	case PDC:
 	  if (cond)





~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Free download of GNUSH tool chain for Hitachi's SH Series.
The following site also offers free support to European customers.
Read more at http://www.kpit.com/products/support.htm.
Latest version of GNUSH is released on Jan 1, 2002.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 

Re: [PATCH] : SH Assembler generates incorrect opcode for PCMP instructions

[Nick Clifton]

Hi Arati,

> The SH assembler generates incorrect opcodes for the parallel PCMP 
> instruction.  Basically, its last nibble is a copy of the previous
> instruction's second nibble when working in Big Endian format.
> 
> For example,
> test.s
> 	movs.w @-R5, A0
>       PCMP X0,Y0
> 
> generates opcodes
> 	f5 70
> 	f8 00 84 05
> 
>  instead of
> 	f8 00 84 00
> 
> The following patch corrects this. I have also verified that it does
> not cause any side effect on other DSP instructions.

>  	  if (field_b)
>  	    as_bad (_("multiple parallel processing specifications"));
>  	  field_b = ((opcode->nibbles[2] << 12) + (opcode->nibbles[3] <<
> 8)
> -		     + (reg_x << 6) + (reg_y << 4) + reg_n);
> +		     + (reg_x << 6) + (reg_y << 4) );
> +
> +	  if (strcmp (opcode->name, "pcmp") != 0)
> +	  	 field_b += reg_n;
>  	  break;
>  	case PDC:
>  	  if (cond)

Your patch does work, but I do not like the way it makes a special
case out of the opcode's name.  In fact I think that it may not be
just the PCMP instruction that has this problem, since a perusal of
the sh-opc.h file indicates that the PABS and PRND instructions may
also have similar difficulties.

Please could you try the patch below, which I believe provides a more
generic fix for the problem, and which also adds some tests to the SH
gas testsuite to make sure that the problem does not arise again in
the future.

Cheers
        Nick

gas/ChangeLog
2002-01-31  Nick Clifton  <nickc@cambridge.redhat.com>

	* config/tc-sh.c (opcode_has_arg): New function.
        (assemble_ppi): For case PPI3 only insert those registers
        which are specified in the opcode's arg array.

gas/testsuite/ChangeLog
2002-01-31  Nick Clifton  <nickc@cambridge.redhat.com>

	* gas/sh/dsp.s: Add tests of a few DSP instructions.
        * gas/sh/dsp.d: Add expected disassembly.

Index: gas/config/tc-sh.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-sh.c,v
retrieving revision 1.53
diff -c -3 -p -w -r1.53 tc-sh.c
*** tc-sh.c	2002/01/30 18:25:30	1.53
--- tc-sh.c	2002/01/31 21:38:50
***************
*** 29,34 ****
--- 29,35 ----
  #include "opcodes/sh-opc.h"
  #include "safe-ctype.h"
  #include "struc-symbol.h"
+ #include "libiberty.h"
  
  #ifdef OBJ_ELF
  #include "elf/sh.h"
*************** static void build_relax PARAMS ((sh_opco
*** 72,77 ****
--- 73,79 ----
  static char *insert_loop_bounds PARAMS ((char *, sh_operand_info *));
  static unsigned int build_Mytes
    PARAMS ((sh_opcode_info *, sh_operand_info *));
+ static boolean opcode_has_arg PARAMS ((sh_opcode_info *, sh_arg_type));
  
  #ifdef OBJ_ELF
  static void sh_elf_cons PARAMS ((int));
*************** find_cooked_opcode (str_p)
*** 1679,1684 ****
--- 1681,1702 ----
    return (sh_opcode_info *) hash_find (opcode_hash_control, name);
  }
  
+ /* Returns true if OPCODE has an arg of type ARG.  */
+ 
+ static boolean
+ opcode_has_arg (opcode, arg)
+      sh_opcode_info * opcode;
+      sh_arg_type arg;
+ {
+   unsigned int i;
+ 
+   for (i = 0; i < ARRAY_SIZE (opcode->arg); i++)
+     if (opcode->arg[i] == arg)
+       return true;
+ 
+   return false;
+ }
+ 
  /* Assemble a parallel processing insn.  */
  #define DDT_BASE 0xf000 /* Base value for double data transfer insns */
  
*************** assemble_ppi (op_end, opcode)
*** 1791,1798 ****
  	case PPI3:
  	  if (field_b)
  	    as_bad (_("multiple parallel processing specifications"));
! 	  field_b = ((opcode->nibbles[2] << 12) + (opcode->nibbles[3] << 8)
! 		     + (reg_x << 6) + (reg_y << 4) + reg_n);
  	  break;
  	case PDC:
  	  if (cond)
--- 1809,1824 ----
  	case PPI3:
  	  if (field_b)
  	    as_bad (_("multiple parallel processing specifications"));
!  	  field_b = ((opcode->nibbles[2] << 12) + (opcode->nibbles[3] << 8));
! 
! 	  if (opcode_has_arg (opcode, DSP_REG_X))
! 	    field_b |= (reg_x << 6);
! 
! 	  if (opcode_has_arg (opcode, DSP_REG_Y))
! 	    field_b |= (reg_y << 4);
!       
! 	  if (opcode_has_arg (opcode, DSP_REG_N))
! 	    field_b |= reg_n;
  	  break;
  	case PDC:
  	  if (cond)

Index: gas/testsuite/gas/sh/dsp.s
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/sh/dsp.s,v
retrieving revision 1.1
diff -c -3 -p -w -r1.1 dsp.s
*** dsp.s	2001/10/09 12:25:52	1.1
--- dsp.s	2002/01/31 21:38:50
***************
*** 1,4 ****
! #	Test file for ARM/GAS -- basic instructions
  
  	.text
  	.align
--- 1,4 ----
! #	Test file for SH DSP assembler instructions.
  
  	.text
  	.align
*************** dsp_tests:
*** 22,24 ****
--- 22,36 ----
  	movs.l m0, @r3+
  	movs.l m1, @r2+r8
  
+ 	# The following tests reproduce a bug discovered in the toolchain
+ 	# The symptom was that insns encoded with the PPI3 attribute would
+ 	# always have three registers encoded into their bitfields, even if
+ 	# they did not actually accept three registers.  The "third" register
+ 	# would come from whatever value was used for the previous instruction.
+ 	movs.w @-R5, A0
+ 	pcmp   X0, Y0
+ 	psubc  X1, Y1, A1
+ 	pabs   X0, A0
+ 	pabs   Y0, A1
+ 	prnd   X1, M0
+ 	prnd   Y1, A1G

Index: gas/testsuite/gas/sh/dsp.d
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/sh/dsp.d,v
retrieving revision 1.2
diff -c -3 -p -w -r1.2 dsp.d
*** dsp.d	2002/01/15 12:27:53	1.2
--- dsp.d	2002/01/31 21:38:50
*************** Disassembly of section .text:
*** 22,24 ****
--- 22,31 ----
  0+01a <[^>]*> f4 b7 [ 	]*movs.l	y1,@r4
  0+01c <[^>]*> f7 cb [ 	]*movs.l	m0,@r3\+
  0+01e <[^>]*> f6 ef [ 	]*movs.l	m1,@r2\+r8
+ 0+020 <[^>]*> f5 70 [ 	]*movs.w	@-r5,a0
+ 0+022 <[^>]*> f8 00 84 00 [ 	]*pcmp	x0,y0
+ 0+026 <[^>]*> f8 00 a0 55 [ 	]*psubc	x1,y1,a1
+ 0+02a <[^>]*> f8 00 88 07 [ 	]*pabs	x0,a0
+ 0+02e <[^>]*> f8 00 a8 05 [ 	]*pabs	y0,a1
+ 0+032 <[^>]*> f8 00 98 4c [ 	]*prnd	x1,m0
+ 0+036 <[^>]*> f8 00 b8 1d [ 	]*prnd	y1,a1g

Re: [PATCH] : SH Assembler generates incorrect opcode for PCMP instructions

[Joern Rennecke]

Nick Clifton wrote:
> 
> Hi Arati,
> 
> > The SH assembler generates incorrect opcodes for the parallel PCMP
> > instruction.  Basically, its last nibble is a copy of the previous
> > instruction's second nibble when working in Big Endian format.

That's not exactly incorrect, since the reg_n field for pcmp is
ignored by the CPU.

> Your patch does work, but I do not like the way it makes a special
> case out of the opcode's name.  In fact I think that it may not be
> just the PCMP instruction that has this problem, since a perusal of
> the sh-opc.h file indicates that the PABS and PRND instructions may
> also have similar difficulties.

No, PCMP is indeed special, since it is the only PPI3 instuction that
ignores REG_N.

Moreover, REG_X and REG_Y are initialized at the start of assemble_ppi.

To get non-varying opcodes for pcmp, you can clear reg_n at that point, too.
	
-- 
--------------------------
SuperH
2430 Aztec West / Almondsbury / BRISTOL / BS32 4AQ
T:+44 1454 462330

[ PATCH] : SH Assembler generates incorrect opcode for PCMP instructions

[Arati Dikey]

Hi !

The SH assembler generates incorrect opcodes for the parallel PCMP
instruction.
Basically, its last nibble is a copy of the previous instruction's
second nibble
when working in Big Endian format.

For example,
test.s
	movs.w @-R5, A0
      PCMP X0,Y0

 generates opcodes
	f5 70
	f8 00 84 05

 instead of
	f8 00 84 00

The following patch corrects this. I have also verified that it does not
cause any
side effect on other DSP instructions.

Regards,
Arati Dikey



--- tc-sh.c.orig	Thu Dec  6 11:34:18 2001
+++ tc-sh.c	      Thu Jan 17 16:41:28 2002
@@ -1769,7 +1769,10 @@
 	  if (field_b)
 	    as_bad (_("multiple parallel processing specifications"));
 	  field_b = ((opcode->nibbles[2] << 12) + (opcode->nibbles[3] <<
8)
-		     + (reg_x << 6) + (reg_y << 4) + reg_n);
+		     + (reg_x << 6) + (reg_y << 4) );
+
+	  if (strcmp (opcode->name, "pcmp") != 0)
+	  	 field_b += reg_n;
 	  break;
 	case PDC:
 	  if (cond)