public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
* CGEN patch
@ 2000-12-21  8:24 Richard Sandiford
  2000-12-21 10:45 ` Frank Ch. Eigler
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Richard Sandiford @ 2000-12-21  8:24 UTC (permalink / raw)
  To: binutils, cgen

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/

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: CGEN patch
@ 2000-12-21 10:20 Nick Clifton
  0 siblings, 0 replies; 8+ messages in thread
From: Nick Clifton @ 2000-12-21 10:20 UTC (permalink / raw)
  To: rsandifo; +Cc: binutils, cgen

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2001-01-03  7:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2000-12-21  8:24 CGEN patch Richard Sandiford
2000-12-21 10:45 ` Frank Ch. Eigler
2000-12-21 14:04 ` Doug Evans
2001-01-02  6:51   ` Richard Sandiford
2001-01-02  8:34     ` Frank Ch. Eigler
2001-01-03  2:52 ` Richard Sandiford
2001-01-03  7:10   ` Frank Ch. Eigler
2000-12-21 10:20 Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).