CGEN patch

CGEN patch

[Richard Sandiford]

This is a cgen-related binutils patch.  Uncertain of the etiquette, I'm
posting it to both the cgen and binutils lists.  This is my first
binutils patch... you have been warned!

cgen has functions for converting a general value to and from a byte
array.  The functions take the size of the value and its endianness as
arguments.  Previously, they catered only for 8, 16 and 32 bit values:
this patch makes them work with 24-bit values as well.  Because the
functions could be useful to a wider audience, and because they seem to
fit naturally into the bfd_getl16() family, I've moved them into
libbfd.c.

On a related issue, extract_normal() only sign-extends a value if it
CGEN_INT_INSN_P is true.  The patch makes it do sign extension to all
cases.

I've split the patch into two.   First the bfd part, then the opcodes part.

Ran "make check" on the fr30 port without regressions.

Please install if patch is OK.

Richard


------ BFD patch

2000-12-21  Richard Sandiford  <rsandifo@redhat.com>

	* libbfd.c (bfd_get_bits): Added
	(bfd_put_bits): Likewise
	* bfd-in.h: Declared the above.

Index: bfd/bfd-in.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in.h,v
retrieving revision 1.22
diff -c -r1.22 bfd-in.h
*** bfd-in.h	2000/11/28 21:42:15	1.22
--- bfd/bfd-in.h	2000/12/21 16:12:47
***************
*** 538,543 ****
--- 538,549 ----
  void		bfd_putl32	   PARAMS ((bfd_vma, unsigned char *));
  void		bfd_putb16	   PARAMS ((bfd_vma, unsigned char *));
  void		bfd_putl16	   PARAMS ((bfd_vma, unsigned char *));
+ 
+ /* Byte swapping routines which take size and endiannes as arguments. */
+ 
+ bfd_vma bfd_get_bits PARAMS ((bfd_byte *, int, boolean));
+ void bfd_put_bits PARAMS ((bfd_vma, bfd_byte *, int, boolean));
+ 
  \f
  /* Externally visible ECOFF routines.  */
  
Index: bfd/libbfd.c
===================================================================
RCS file: /cvs/src/src/bfd/libbfd.c,v
retrieving revision 1.12
diff -c -r1.12 libbfd.c
*** libbfd.c	2000/07/31 18:49:56	1.12
--- bfd/libbfd.c	2000/12/21 16:12:48
***************
*** 1190,1195 ****
--- 1190,1240 ----
    BFD_FAIL();
  #endif
  }
+ 
+ void
+ bfd_put_bits (data, addr, bits, big_p)
+      bfd_vma data;
+      bfd_byte *addr;
+      int bits;
+      boolean big_p;
+ {
+   int i;
+   int bytes;
+ 
+   if (bits % 8 != 0)
+     abort();
+ 
+   bytes = bits / 8;
+   for (i = 0; i < bytes; i++)
+     {
+       int index = big_p? bytes - i - 1 : i;
+       addr[index] = (bfd_byte) data;
+       data >>= 8;
+     }
+ }
+ 
+ bfd_vma
+ bfd_get_bits (addr, bits, big_p)
+      bfd_byte *addr;
+      int bits;
+      boolean big_p;
+ {
+   bfd_vma data;
+   int i;
+   int bytes;
+ 
+   if (bits % 8 != 0)
+     abort();
+ 
+   data = 0;
+   bytes = bits / 8;
+   for (i = 0; i < bytes; i++)
+     {
+       int index = big_p? i : bytes - i - 1;
+       data = (data << 8) | addr[index];
+     }
+   return data;
+ }
  \f
  /* Default implementation */
  


------ OPCODES patch

2000-12-21  Richard Sandiford  <rsandifo@redhat.com>

	* cgen-dis.c (hash_insn_array): Use bfd_put_bits().
	(hash_insn_list): Likewise
	* cgen-ibld.in (insert_1): Use bfd_put_bits() and bfd_get_bits().
	(extract_1): Use bfd_get_bits().
	(extract_normal): Apply sign extension to both extraction
	methods.
	* include/opcode/cgen.h (cgen_get_insn_value): Now a macro
	(cgen_put_insn_value): Likewise
	* cgen-op.c (cgen_get_insn_value): Removed
	(cgen_put_insn_value): Likewise

Index: opcodes/cgen-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-dis.c,v
retrieving revision 1.2
diff -c -r1.2 cgen-dis.c
*** cgen-dis.c	2000/07/26 22:45:49	1.2
--- opcodes/cgen-dis.c	2000/12/21 16:12:48
***************
*** 64,90 ****
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (insn);
!       switch (CGEN_INSN_MASK_BITSIZE (insn))
! 	{
! 	case 8:
! 	  buf[0] = value;
! 	  break;
! 	case 16:
! 	  if (big_p)
! 	    bfd_putb16 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl16 ((bfd_vma) value, buf);
! 	  break;
! 	case 32:
! 	  if (big_p)
! 	    bfd_putb32 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl32 ((bfd_vma) value, buf);
! 	  break;
! 	default:
! 	  abort ();
! 	}
! 
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable[hash];
        hentbuf->insn = insn;
--- 64,73 ----
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (insn);
!       bfd_put_bits ((bfd_vma) value,
! 		    buf,
! 		    CGEN_INSN_MASK_BITSIZE (insn),
! 		    big_p);
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable[hash];
        hentbuf->insn = insn;
***************
*** 121,147 ****
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (ilist->insn);
!       switch (CGEN_INSN_MASK_BITSIZE (ilist->insn))
! 	{
! 	case 8:
! 	  buf[0] = value;
! 	  break;
! 	case 16:
! 	  if (big_p)
! 	    bfd_putb16 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl16 ((bfd_vma) value, buf);
! 	  break;
! 	case 32:
! 	  if (big_p)
! 	    bfd_putb32 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl32 ((bfd_vma) value, buf);
! 	  break;
! 	default:
! 	  abort ();
! 	}
! 
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable [hash];
        hentbuf->insn = ilist->insn;
--- 104,113 ----
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (ilist->insn);
!       bfd_put_bits((bfd_vma) value,
! 		   buf,
! 		   CGEN_INSN_MASK_BITSIZE (ilist->insn),
! 		   big_p);
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable [hash];
        hentbuf->insn = ilist->insn;
Index: opcodes/cgen-ibld.in
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-ibld.in,v
retrieving revision 1.2
diff -c -r1.2 cgen-ibld.in
*** cgen-ibld.in	2000/08/28 18:17:54	1.2
--- opcodes/cgen-ibld.in	2000/12/21 16:12:49
***************
*** 78,111 ****
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   switch (word_length)
!     {
!     case 8:
!       x = *bufp;
!       break;
!     case 16:
!       if (big_p)
! 	x = bfd_getb16 (bufp);
!       else
! 	x = bfd_getl16 (bufp);
!       break;
!     case 24:
!       /* ??? This may need reworking as these cases don't necessarily
! 	 want the first byte and the last two bytes handled like this.  */
!       if (big_p)
! 	x = (bufp[0] << 16) | bfd_getb16 (bufp + 1);
!       else
! 	x = bfd_getl16 (bufp) | (bufp[2] << 16);
!       break;
!     case 32:
!       if (big_p)
! 	x = bfd_getb32 (bufp);
!       else
! 	x = bfd_getl32 (bufp);
!       break;
!     default :
!       abort ();
!     }
  
    /* Written this way to avoid undefined behaviour.  */
    mask = (((1L << (length - 1)) - 1) << 1) | 1;
--- 78,84 ----
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   x = bfd_get_bits (bufp, word_length, big_p);
  
    /* Written this way to avoid undefined behaviour.  */
    mask = (((1L << (length - 1)) - 1) << 1) | 1;
***************
*** 115,154 ****
      shift = (word_length - (start + length));
    x = (x & ~(mask << shift)) | ((value & mask) << shift);
  
!   switch (word_length)
!     {
!     case 8:
!       *bufp = x;
!       break;
!     case 16:
!       if (big_p)
! 	bfd_putb16 (x, bufp);
!       else
! 	bfd_putl16 (x, bufp);
!       break;
!     case 24:
!       /* ??? This may need reworking as these cases don't necessarily
! 	 want the first byte and the last two bytes handled like this.  */
!       if (big_p)
! 	{
! 	  bufp[0] = x >> 16;
! 	  bfd_putb16 (x, bufp + 1);
! 	}
!       else
! 	{
! 	  bfd_putl16 (x, bufp);
! 	  bufp[2] = x >> 16;
! 	}
!       break;
!     case 32:
!       if (big_p)
! 	bfd_putb32 (x, bufp);
!       else
! 	bfd_putl32 (x, bufp);
!       break;
!     default :
!       abort ();
!     }
  }
  
  #endif /* ! CGEN_INT_INSN_P */
--- 88,94 ----
      shift = (word_length - (start + length));
    x = (x & ~(mask << shift)) | ((value & mask) << shift);
  
!   bfd_put_bits ((bfd_vma) x, bufp, word_length, big_p);
  }
  
  #endif /* ! CGEN_INT_INSN_P */
***************
*** 406,451 ****
       unsigned char *bufp;
       bfd_vma pc;
  {
!   unsigned long x,mask;
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   switch (word_length)
!     {
!     case 8:
!       x = *bufp;
!       break;
!     case 16:
!       if (big_p)
! 	x = bfd_getb16 (bufp);
!       else
! 	x = bfd_getl16 (bufp);
!       break;
!     case 24:
!       /* ??? This may need reworking as these cases don't necessarily
! 	 want the first byte and the last two bytes handled like this.  */
!       if (big_p)
! 	x = (bufp[0] << 16) | bfd_getb16 (bufp + 1);
!       else
! 	x = bfd_getl16 (bufp) | (bufp[2] << 16);
!       break;
!     case 32:
!       if (big_p)
! 	x = bfd_getb32 (bufp);
!       else
! 	x = bfd_getl32 (bufp);
!       break;
!     default :
!       abort ();
!     }
  
-   /* Written this way to avoid undefined behaviour.  */
-   mask = (((1L << (length - 1)) - 1) << 1) | 1;
    if (CGEN_INSN_LSB0_P)
      shift = (start + 1) - length;
    else
      shift = (word_length - (start + length));
!   return (x >> shift) & mask;
  }
  
  #endif /* ! CGEN_INT_INSN_P */
--- 346,362 ----
       unsigned char *bufp;
       bfd_vma pc;
  {
!   unsigned long x;
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   x = bfd_get_bits (bufp, word_length, big_p);
  
    if (CGEN_INSN_LSB0_P)
      shift = (start + 1) - length;
    else
      shift = (word_length - (start + length));
!   return x >> shift;
  }
  
  #endif /* ! CGEN_INT_INSN_P */
***************
*** 489,495 ****
  #endif
       long *valuep;
  {
!   CGEN_INSN_INT value;
  
    /* If LENGTH is zero, this operand doesn't contribute to the value
       so give it a standard value of zero.  */
--- 400,406 ----
  #endif
       long *valuep;
  {
!   CGEN_INSN_INT value, mask;
  
    /* If LENGTH is zero, this operand doesn't contribute to the value
       so give it a standard value of zero.  */
***************
*** 521,538 ****
  
    if (CGEN_INT_INSN_P || word_offset == 0)
      {
-       /* Written this way to avoid undefined behaviour.  */
-       CGEN_INSN_INT mask = (((1L << (length - 1)) - 1) << 1) | 1;
- 
        if (CGEN_INSN_LSB0_P)
  	value = insn_value >> ((word_offset + start + 1) - length);
        else
  	value = insn_value >> (total_length - ( word_offset + start + length));
-       value &= mask;
-       /* sign extend? */
-       if (CGEN_BOOL_ATTR (attrs, CGEN_IFLD_SIGNED)
- 	  && (value & (1L << (length - 1))))
- 	value |= ~mask;
      }
  
  #if ! CGEN_INT_INSN_P
--- 432,441 ----
***************
*** 551,556 ****
--- 454,468 ----
      }
  
  #endif /* ! CGEN_INT_INSN_P */
+ 
+   /* Written this way to avoid undefined behaviour.  */
+   mask = (((1L << (length - 1)) - 1) << 1) | 1;
+ 
+   value &= mask;
+   /* sign extend? */
+   if (CGEN_BOOL_ATTR (attrs, CGEN_IFLD_SIGNED)
+       && (value & (1L << (length - 1))))
+     value |= ~mask;
  
    *valuep = value;
  
Index: opcodes/cgen-opc.c
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-opc.c,v
retrieving revision 1.3
diff -c -r1.3 cgen-opc.c
*** cgen-opc.c	2000/07/26 22:45:49	1.3
--- opcodes/cgen-opc.c	2000/12/21 16:12:49
***************
*** 361,431 ****
  
    return count;
  }
- 
- /* Cover function to read and properly byteswap an insn value.  */
- 
- CGEN_INSN_INT
- cgen_get_insn_value (cd, buf, length)
-      CGEN_CPU_DESC cd;
-      unsigned char *buf;
-      int length;
- {
-   CGEN_INSN_INT value;
- 
-   switch (length)
-     {
-     case 8:
-       value = *buf;
-       break;
-     case 16:
-       if (cd->insn_endian == CGEN_ENDIAN_BIG)
- 	value = bfd_getb16 (buf);
-       else
- 	value = bfd_getl16 (buf);
-       break;
-     case 32:
-       if (cd->insn_endian == CGEN_ENDIAN_BIG)
- 	value = bfd_getb32 (buf);
-       else
- 	value = bfd_getl32 (buf);
-       break;
-     default:
-       abort ();
-     }
- 
-   return value;
- }
- 
- /* Cover function to store an insn value properly byteswapped.  */
- 
- void
- cgen_put_insn_value (cd, buf, length, value)
-      CGEN_CPU_DESC cd;
-      unsigned char *buf;
-      int length;
-      CGEN_INSN_INT value;
- {
-   switch (length)
-     {
-     case 8:
-       buf[0] = value;
-       break;
-     case 16:
-       if (cd->insn_endian == CGEN_ENDIAN_BIG)
- 	bfd_putb16 (value, buf);
-       else
- 	bfd_putl16 (value, buf);
-       break;
-     case 32:
-       if (cd->insn_endian == CGEN_ENDIAN_BIG)
- 	bfd_putb32 (value, buf);
-       else
- 	bfd_putl32 (value, buf);
-       break;
-     default:
-       abort ();
-     }
- }
  \f
  /* Look up instruction INSN_*_VALUE and extract its fields.
     INSN_INT_VALUE is used if CGEN_INT_INSN_P.
--- 361,366 ----
Index: include/opcode/cgen.h
===================================================================
RCS file: /cvs/src/src/include/opcode/cgen.h,v
retrieving revision 1.7
diff -c -r1.7 cgen.h
*** cgen.h	2000/07/26 22:44:42	1.7
--- include/opcode/cgen.h	2000/12/21 16:12:51
***************
*** 1391,1400 ****
  
  /* Cover fns to bfd_get/set.  */
  
! extern CGEN_INSN_INT cgen_get_insn_value
!      PARAMS ((CGEN_CPU_DESC, unsigned char *, int));
! extern void cgen_put_insn_value
!      PARAMS ((CGEN_CPU_DESC, unsigned char *, int, CGEN_INSN_INT));
  
  /* Read in a cpu description file.
     ??? For future concerns, including adding instructions to the assembler/
--- 1391,1401 ----
  
  /* Cover fns to bfd_get/set.  */
  
! #define cgen_get_insn_value(CP, BUF, LENGTH) \
! 	bfd_get_bits ((BUF), (LENGTH), (CP)->insn_endian == CGEN_ENDIAN_BIG)
! #define cgen_put_insn_value(CP, BUF, LENGTH, VALUE) \
! 	bfd_put_bits ((bfd_vma) (VALUE), (BUF), (LENGTH),\
! 		      (CP)->insn_endian == CGEN_ENDIAN_BIG)
  
  /* Read in a cpu description file.
     ??? For future concerns, including adding instructions to the assembler/

Re: CGEN patch

[Frank Ch. Eigler]

Hi -

On Thu, Dec 21, 2000 at 04:24:04PM +0000, Richard Sandiford wrote:
: [...]
: This is a cgen-related binutils patch.  [...]
: Ran "make check" on the fr30 port without regressions.

Thanks, it looks good on paper!

It would also be nice to test with ports that stress weaker cgen features
like variable-length instructions, but there appear to be no such ports yet
externally released.

- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6Qk/WVZbdDOm/ZT0RAv3eAKCCP8rr6ifgPqI+p4jnlsn0VVFgkgCfWmKh
p1g5ErZZitsA5zdnNpjjmiM=
=BOrr
-----END PGP SIGNATURE-----

CGEN patch

[Doug Evans]

Richard Sandiford writes:
 > 2000-12-21  Richard Sandiford  <rsandifo@redhat.com>
 > 
 > 	* cgen-dis.c (hash_insn_array): Use bfd_put_bits().
 > 	(hash_insn_list): Likewise
 > 	* cgen-ibld.in (insert_1): Use bfd_put_bits() and bfd_get_bits().
 > 	(extract_1): Use bfd_get_bits().
 > 	(extract_normal): Apply sign extension to both extraction
 > 	methods.
 > 	* include/opcode/cgen.h (cgen_get_insn_value): Now a macro
 > 	(cgen_put_insn_value): Likewise
 > 	* cgen-op.c (cgen_get_insn_value): Removed
 > 	(cgen_put_insn_value): Likewise

fwiw, I'd prefer to keep cgen_get/put_insn_value kept as functions.
They can still call the bfd routines, but I'd rather not
have them as functions.

Re: CGEN patch

[Richard Sandiford]

On Thu, 21 Dec 2000, Doug Evans wrote:

> fwiw, I'd prefer to keep cgen_get/put_insn_value kept as functions.
> They can still call the bfd routines, but I'd rather not
> have them as functions.
> 

OK, revised patch below.

2001-01-02  Richard Sandiford  <rsandifo@redhat.com>

	* cgen-dis.c (hash_insn_array): Use bfd_put_bits().
	(hash_insn_list): Likewise
	* cgen-ibld.in (insert_1): Use bfd_put_bits() and bfd_get_bits().
	(extract_1): Use bfd_get_bits().
	(extract_normal): Apply sign extension to both extraction
	methods.
	* cgen-op.c (cgen_get_insn_value): Use bfd_get_bits()
	(cgen_put_insn_value): Use bfd_put_bits()

Index: opcodes/cgen-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-dis.c,v
retrieving revision 1.2
diff -c -r1.2 cgen-dis.c
*** cgen-dis.c	2000/07/26 22:45:49	1.2
--- opcodes/cgen-dis.c	2001/01/02 14:36:44
***************
*** 64,90 ****
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (insn);
!       switch (CGEN_INSN_MASK_BITSIZE (insn))
! 	{
! 	case 8:
! 	  buf[0] = value;
! 	  break;
! 	case 16:
! 	  if (big_p)
! 	    bfd_putb16 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl16 ((bfd_vma) value, buf);
! 	  break;
! 	case 32:
! 	  if (big_p)
! 	    bfd_putb32 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl32 ((bfd_vma) value, buf);
! 	  break;
! 	default:
! 	  abort ();
! 	}
! 
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable[hash];
        hentbuf->insn = insn;
--- 64,73 ----
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (insn);
!       bfd_put_bits ((bfd_vma) value,
! 		    buf,
! 		    CGEN_INSN_MASK_BITSIZE (insn),
! 		    big_p);
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable[hash];
        hentbuf->insn = insn;
***************
*** 121,147 ****
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (ilist->insn);
!       switch (CGEN_INSN_MASK_BITSIZE (ilist->insn))
! 	{
! 	case 8:
! 	  buf[0] = value;
! 	  break;
! 	case 16:
! 	  if (big_p)
! 	    bfd_putb16 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl16 ((bfd_vma) value, buf);
! 	  break;
! 	case 32:
! 	  if (big_p)
! 	    bfd_putb32 ((bfd_vma) value, buf);
! 	  else
! 	    bfd_putl32 ((bfd_vma) value, buf);
! 	  break;
! 	default:
! 	  abort ();
! 	}
! 
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable [hash];
        hentbuf->insn = ilist->insn;
--- 104,113 ----
  	 to hash on, so set both up.  */
  
        value = CGEN_INSN_BASE_VALUE (ilist->insn);
!       bfd_put_bits((bfd_vma) value,
! 		   buf,
! 		   CGEN_INSN_MASK_BITSIZE (ilist->insn),
! 		   big_p);
        hash = (* cd->dis_hash) (buf, value);
        hentbuf->next = htable [hash];
        hentbuf->insn = ilist->insn;
Index: opcodes/cgen-opc.c
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-opc.c,v
retrieving revision 1.4
diff -c -r1.4 cgen-opc.c
*** cgen-opc.c	2000/12/21 18:43:33	1.4
--- opcodes/cgen-opc.c	2001/01/02 14:36:46
***************
*** 374,403 ****
       unsigned char *buf;
       int length;
  {
!   CGEN_INSN_INT value;
! 
!   switch (length)
!     {
!     case 8:
!       value = *buf;
!       break;
!     case 16:
!       if (cd->insn_endian == CGEN_ENDIAN_BIG)
! 	value = bfd_getb16 (buf);
!       else
! 	value = bfd_getl16 (buf);
!       break;
!     case 32:
!       if (cd->insn_endian == CGEN_ENDIAN_BIG)
! 	value = bfd_getb32 (buf);
!       else
! 	value = bfd_getl32 (buf);
!       break;
!     default:
!       abort ();
!     }
! 
!   return value;
  }
  
  /* Cover function to store an insn value properly byteswapped.  */
--- 374,380 ----
       unsigned char *buf;
       int length;
  {
!   bfd_get_bits (buf, length, cd->insn_endian == CGEN_ENDIAN_BIG);
  }
  
  /* Cover function to store an insn value properly byteswapped.  */
***************
*** 409,434 ****
       int length;
       CGEN_INSN_INT value;
  {
!   switch (length)
!     {
!     case 8:
!       buf[0] = value;
!       break;
!     case 16:
!       if (cd->insn_endian == CGEN_ENDIAN_BIG)
! 	bfd_putb16 (value, buf);
!       else
! 	bfd_putl16 (value, buf);
!       break;
!     case 32:
!       if (cd->insn_endian == CGEN_ENDIAN_BIG)
! 	bfd_putb32 (value, buf);
!       else
! 	bfd_putl32 (value, buf);
!       break;
!     default:
!       abort ();
!     }
  }
  \f
  /* Look up instruction INSN_*_VALUE and extract its fields.
--- 386,393 ----
       int length;
       CGEN_INSN_INT value;
  {
!   bfd_put_bits ((bfd_vma) value, buf, length,
! 		cd->insn_endian == CGEN_ENDIAN_BIG);
  }
  \f
  /* Look up instruction INSN_*_VALUE and extract its fields.
Index: opcodes/cgen-ibld.in
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-ibld.in,v
retrieving revision 1.2
diff -c -r1.2 cgen-ibld.in
*** cgen-ibld.in	2000/08/28 18:17:54	1.2
--- opcodes/cgen-ibld.in	2001/01/02 14:36:47
***************
*** 78,111 ****
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   switch (word_length)
!     {
!     case 8:
!       x = *bufp;
!       break;
!     case 16:
!       if (big_p)
! 	x = bfd_getb16 (bufp);
!       else
! 	x = bfd_getl16 (bufp);
!       break;
!     case 24:
!       /* ??? This may need reworking as these cases don't necessarily
! 	 want the first byte and the last two bytes handled like this.  */
!       if (big_p)
! 	x = (bufp[0] << 16) | bfd_getb16 (bufp + 1);
!       else
! 	x = bfd_getl16 (bufp) | (bufp[2] << 16);
!       break;
!     case 32:
!       if (big_p)
! 	x = bfd_getb32 (bufp);
!       else
! 	x = bfd_getl32 (bufp);
!       break;
!     default :
!       abort ();
!     }
  
    /* Written this way to avoid undefined behaviour.  */
    mask = (((1L << (length - 1)) - 1) << 1) | 1;
--- 78,84 ----
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   x = bfd_get_bits (bufp, word_length, big_p);
  
    /* Written this way to avoid undefined behaviour.  */
    mask = (((1L << (length - 1)) - 1) << 1) | 1;
***************
*** 115,154 ****
      shift = (word_length - (start + length));
    x = (x & ~(mask << shift)) | ((value & mask) << shift);
  
!   switch (word_length)
!     {
!     case 8:
!       *bufp = x;
!       break;
!     case 16:
!       if (big_p)
! 	bfd_putb16 (x, bufp);
!       else
! 	bfd_putl16 (x, bufp);
!       break;
!     case 24:
!       /* ??? This may need reworking as these cases don't necessarily
! 	 want the first byte and the last two bytes handled like this.  */
!       if (big_p)
! 	{
! 	  bufp[0] = x >> 16;
! 	  bfd_putb16 (x, bufp + 1);
! 	}
!       else
! 	{
! 	  bfd_putl16 (x, bufp);
! 	  bufp[2] = x >> 16;
! 	}
!       break;
!     case 32:
!       if (big_p)
! 	bfd_putb32 (x, bufp);
!       else
! 	bfd_putl32 (x, bufp);
!       break;
!     default :
!       abort ();
!     }
  }
  
  #endif /* ! CGEN_INT_INSN_P */
--- 88,94 ----
      shift = (word_length - (start + length));
    x = (x & ~(mask << shift)) | ((value & mask) << shift);
  
!   bfd_put_bits ((bfd_vma) x, bufp, word_length, big_p);
  }
  
  #endif /* ! CGEN_INT_INSN_P */
***************
*** 406,451 ****
       unsigned char *bufp;
       bfd_vma pc;
  {
!   unsigned long x,mask;
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   switch (word_length)
!     {
!     case 8:
!       x = *bufp;
!       break;
!     case 16:
!       if (big_p)
! 	x = bfd_getb16 (bufp);
!       else
! 	x = bfd_getl16 (bufp);
!       break;
!     case 24:
!       /* ??? This may need reworking as these cases don't necessarily
! 	 want the first byte and the last two bytes handled like this.  */
!       if (big_p)
! 	x = (bufp[0] << 16) | bfd_getb16 (bufp + 1);
!       else
! 	x = bfd_getl16 (bufp) | (bufp[2] << 16);
!       break;
!     case 32:
!       if (big_p)
! 	x = bfd_getb32 (bufp);
!       else
! 	x = bfd_getl32 (bufp);
!       break;
!     default :
!       abort ();
!     }
  
-   /* Written this way to avoid undefined behaviour.  */
-   mask = (((1L << (length - 1)) - 1) << 1) | 1;
    if (CGEN_INSN_LSB0_P)
      shift = (start + 1) - length;
    else
      shift = (word_length - (start + length));
!   return (x >> shift) & mask;
  }
  
  #endif /* ! CGEN_INT_INSN_P */
--- 346,362 ----
       unsigned char *bufp;
       bfd_vma pc;
  {
!   unsigned long x;
    int shift;
    int big_p = CGEN_CPU_INSN_ENDIAN (cd) == CGEN_ENDIAN_BIG;
  
!   x = bfd_get_bits (bufp, word_length, big_p);
  
    if (CGEN_INSN_LSB0_P)
      shift = (start + 1) - length;
    else
      shift = (word_length - (start + length));
!   return x >> shift;
  }
  
  #endif /* ! CGEN_INT_INSN_P */
***************
*** 489,495 ****
  #endif
       long *valuep;
  {
!   CGEN_INSN_INT value;
  
    /* If LENGTH is zero, this operand doesn't contribute to the value
       so give it a standard value of zero.  */
--- 400,406 ----
  #endif
       long *valuep;
  {
!   CGEN_INSN_INT value, mask;
  
    /* If LENGTH is zero, this operand doesn't contribute to the value
       so give it a standard value of zero.  */
***************
*** 521,538 ****
  
    if (CGEN_INT_INSN_P || word_offset == 0)
      {
-       /* Written this way to avoid undefined behaviour.  */
-       CGEN_INSN_INT mask = (((1L << (length - 1)) - 1) << 1) | 1;
- 
        if (CGEN_INSN_LSB0_P)
  	value = insn_value >> ((word_offset + start + 1) - length);
        else
  	value = insn_value >> (total_length - ( word_offset + start + length));
-       value &= mask;
-       /* sign extend? */
-       if (CGEN_BOOL_ATTR (attrs, CGEN_IFLD_SIGNED)
- 	  && (value & (1L << (length - 1))))
- 	value |= ~mask;
      }
  
  #if ! CGEN_INT_INSN_P
--- 432,441 ----
***************
*** 551,556 ****
--- 454,468 ----
      }
  
  #endif /* ! CGEN_INT_INSN_P */
+ 
+   /* Written this way to avoid undefined behaviour.  */
+   mask = (((1L << (length - 1)) - 1) << 1) | 1;
+ 
+   value &= mask;
+   /* sign extend? */
+   if (CGEN_BOOL_ATTR (attrs, CGEN_IFLD_SIGNED)
+       && (value & (1L << (length - 1))))
+     value |= ~mask;
  
    *valuep = value;
  

Re: CGEN patch

[Frank Ch. Eigler]

Hi -

On Tue, Jan 02, 2001 at 02:50:59PM +0000, Richard Sandiford wrote:
: OK, revised patch below.
: 
: 2001-01-02  Richard Sandiford  <rsandifo@redhat.com>
: 
: 	* cgen-dis.c (hash_insn_array): Use bfd_put_bits().
: 	(hash_insn_list): Likewise
: 	* cgen-ibld.in (insert_1): Use bfd_put_bits() and bfd_get_bits().
: 	(extract_1): Use bfd_get_bits().
: 	(extract_normal): Apply sign extension to both extraction
: 	methods.
: 	* cgen-op.c (cgen_get_insn_value): Use bfd_get_bits()
: 	(cgen_put_insn_value): Use bfd_put_bits()
: [...]

Thanks, looks good.  I committed this for you into the sourceware
repository; the bfd parts were already committed by someone else.


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6UgMnVZbdDOm/ZT0RAq2jAJ41EzudKO6eLogLyBSXdmnQQS5c+ACfcq2r
r54fHtPP+6TETgFOpsl7OnE=
=Tx3i
-----END PGP SIGNATURE-----

Re: CGEN patch

[Richard Sandiford]

Oops, I missed one.  Checked against fr30.  Please check in if OK.


2001-01-03  Richard Sandiford  <r.sandiford@redhat.com>

	* cgen-dis.in (read_insn): Use bfd_get_bits()

Index: opcodes/cgen-dis.in
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-dis.in,v
retrieving revision 1.3
diff -c -r1.3 cgen-dis.in
*** cgen-dis.in	2000/11/07 17:20:25	1.3
--- cgen-dis.in	2001/01/03 10:49:49
***************
*** 211,231 ****
    ex_info->valid = (1 << buflen) - 1;
    ex_info->insn_bytes = buf;
  
!   switch (buflen)
!     {
!     case 1:
!       *insn_value = buf[0];
!       break;
!     case 2:
!       *insn_value = info->endian == BFD_ENDIAN_BIG ? bfd_getb16 (buf) : bfd_getl16 (buf);
!       break;
!     case 4:
!       *insn_value = info->endian == BFD_ENDIAN_BIG ? bfd_getb32 (buf) : bfd_getl32 (buf);
!       break;
!     default:
!       abort ();
!     }
! 
    return 0;
  }
  
--- 211,217 ----
    ex_info->valid = (1 << buflen) - 1;
    ex_info->insn_bytes = buf;
  
!   *insn_value = bfd_get_bits (buf, buflen * 8, info->endian == BFD_ENDIAN_BIG);
    return 0;
  }
  

Re: CGEN patch

[Frank Ch. Eigler]

Hi -

On Wed, Jan 03, 2001 at 10:52:27AM +0000, Richard Sandiford wrote:
: Oops, I missed one.  Checked against fr30.  Please check in if OK.
: 
: 2001-01-03  Richard Sandiford  <r.sandiford@redhat.com>
: 
: 	* cgen-dis.in (read_insn): Use bfd_get_bits()

Thanks, committed.

- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6U0DkVZbdDOm/ZT0RAsHFAJ0dBpo0TOccOdkqHw3JJqjsAh1+jwCfZhHQ
0zwGaj1K27Nr6JHf2ztpf/U=
=qfzt
-----END PGP SIGNATURE-----

Re: CGEN patch

[Nick Clifton]

Hi Richard,

: This is a cgen-related binutils patch.  Uncertain of the etiquette,
: I'm posting it to both the cgen and binutils lists.

This is the right thing to do.  The bfd parts of the patch need to be
approved by a binutils maintainer and the cgen parts by a cgen
maintainer.  Since the cgen parts require the bfd parts, posting them
together is the right approach.

: 2000-12-21  Richard Sandiford  <rsandifo@redhat.com>
: 
: 	* libbfd.c (bfd_get_bits): Added
: 	(bfd_put_bits): Likewise
: 	* bfd-in.h: Declared the above.

Approved and applied.

A couple of minor formatting points:

: + /* Byte swapping routines which take size and endiannes as arguments. */

Two spaces at the end of a comment.

: + bfd_vma bfd_get_bits PARAMS ((bfd_byte *, int, boolean));
: + void bfd_put_bits PARAMS ((bfd_vma, bfd_byte *, int, boolean));
: + 
:   \f

There is no need for a blank line before the ^L.

: +   if (bits % 8 != 0)
: +     abort();

A space between the function name and the opening parenthesis.

: +       int index = big_p? bytes - i - 1 : i;
: +       addr[index] = (bfd_byte) data;

A blank line between the declaration of variables and the start of the
statements.


One other point - bfd-in.h is used to generate bfd-in2.h, so in your
ChangeLog entry you ought to include:

	  bfd-in2.h: Regenerate.

(And of course if you were checking in this patch yourself you would
need to make sure that you did regenerate bfd-in2.h).

I will leave the cgen parts of you your patch to be approved by a cgen
maintainer.

Cheers
	Nick