public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
* Re: CGEN: RFA: CGEN_INT_INSN_P
       [not found] <Pine.LNX.4.21.0008280502180.7345-100000@moshpit.cygnus.com>
@ 2000-08-28  9:30 ` Dave Brolley
  0 siblings, 0 replies; 4+ messages in thread
From: Dave Brolley @ 2000-08-28  9:30 UTC (permalink / raw)
  To: Ben Elliston; +Cc: cgen

Ben Elliston wrote:

> Dave,
>
>    I ran into a problem with the cgen-based gas/opcodes/binutils/sim port
>    that I am working on. The ISA has some 16 bit insns and some 32 bit
>    insns, so I set default-insn-word-bitsize=16, default-insn-bitsize=16
>    and base-insn-bitsize=16.
>
> Have you been able to determine the difference between all of these
> parameters?  I've lucked out so far, in that all of my ports allow them to
> be the same.  Perhaps if I understood it better, I could improve the
> documentation.

Hi Ben,

The problems don't seem to stem from all these parameters being the same. The
problems are being caused by base-insn-bitsize (16 bits in my case) not being
the same size as CGEN_INSN_INT (32 bits) combined with existence of insns of
different sizes. Basically, there are many places in CGEN where only the bits
represented by base-insn-bitsize get loaded into the integer, but then
CGEN goes on to access the rest of the fields anyway.

I've found another such problem in with the generation of decode.c which I am
trying to fix now.

Dave

Dave

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: CGEN: RFA: CGEN_INT_INSN_P
  2000-08-22 13:22 Dave Brolley
  2000-08-22 13:29 ` Dave Brolley
@ 2000-08-28 11:40 ` Dave Brolley
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Brolley @ 2000-08-28 11:40 UTC (permalink / raw)
  To: cgen

Approved offline by Frank Eigler and Doug Evans -- commited.

Dave

Dave Brolley wrote:

> Hi,
>
> I ran into a problem with the cgen-based gas/opcodes/binutils/sim
> port that I am working on. The ISA has some 16 bit insns and some
> 32 bit insns, so I set default-insn-word-bitsize=16,
> default-insn-bitsize=16 and base-insn-bitsize=16.
>
> Now in <arch>-desc.h, CGEN_INT_INSN_P gets define to 1, since all
> of my insns are 32 bit or less. However, the current code enabled
> by CGEN_INT_INSN_P in cgen-ibld.in and cgen-dis.in does not allow
> for base-insn-bitsize != max-insn-bitsize.
>
> 1) It aborts in 'extract_normal' because when accessing a 16 bit
> insn field at offset 16 it is called with word_offset==16. Also
> start==0, length==16, word_length==16 and total_length==32. These
> value are generated by cgen in <arch>_cgen_insert_operand and I
> believe that they are correct.
>
> 2) Even if the abort did not occur the generated insn was wrong
> because the insns base value was not correctly onserted into the
> insn integer by insert_insn_normal.
>
> 3) A similar problem to 2) exists in print_insn
>
> This patch allows for CGEN_INT_INSN_P==1 with base-insn-bitsize
> != max-insn-bitsize. I have tested it against my correct work
> (which has this configuration) and against the fr30 (which has
> CGEN_INT_INSN_P==0) and against another port for which
> CGEN_INT_INSN_P==1 and base-insn-bitsize == max-insn-bitsize.
>
> OK to commit? ---- or was there a much easier way to handle
> this?  :-)
>
> Dave
>
>   ------------------------------------------------------------------------
> 2000-08-22  Dave Brolley  <brolley@redhat.com>
>
>         * cgen-ibld.in (cgen_put_insn_int_value): New function.
>         (insert_normal): Allow for non-zero word_offset with CGEN_INT_INSN_P.
>         (insert_insn_normal): Use cgen_put_insn_int_value with CGEN_INT_INSN_P.
>         (extract_normal): Allow for non-zero word_offset with CGEN_INT_INSN_P.
>         * cgen-dis.in (read_insn): New statis function.
>         (print_insn): Use read_insn to read the insn into the buffer and set
>         up for disassembly.
>         (print_insn): in CGEN_INT_INSN_P, make sure that the entire insn is
>         in the buffer.
>
>   ------------------------------------------------------------------------
> Index: opcodes/cgen-dis.in
> ===================================================================
> RCS file: /cvs/src/src/opcodes/cgen-dis.in,v
> retrieving revision 1.1
> diff -c -p -r1.1 cgen-dis.in
> *** cgen-dis.in 2000/08/04 02:21:43     1.1
> --- cgen-dis.in 2000/08/22 19:59:17
> *************** print_insn_normal (cd, dis_info, insn, f
> *** 187,229 ****
>       }
>   }
>
> ! /* Utility to print an insn.
> !    BUF is the base part of the insn, target byte order, BUFLEN bytes long.
> !    The result is the size of the insn in bytes or zero for an unknown insn
> !    or -1 if an error occurs fetching data (memory_error_func will have
> !    been called).  */
> !
>   static int
> ! print_insn (cd, pc, info, buf, buflen)
>        CGEN_CPU_DESC cd;
>        bfd_vma pc;
>        disassemble_info *info;
>        char *buf;
>        int buflen;
>   {
> !   unsigned long insn_value;
> !   const CGEN_INSN_LIST *insn_list;
> !   CGEN_EXTRACT_INFO ex_info;
>
> !   ex_info.dis_info = info;
> !   ex_info.valid = (1 << (cd->base_insn_bitsize / 8)) - 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 ();
>       }
>
>     /* The instructions are stored in hash lists.
>        Pick the first one and keep trying until we find the right one.  */
>
> --- 187,256 ----
>       }
>   }
>
> ! /* Subroutine of print_insn. Reads an insn into the given buffers and updates
> !    the extract info.
> !    Returns 0 if all is well, non-zero otherwise.  */
>   static int
> ! read_insn (cd, pc, info, buf, buflen, ex_info, insn_value)
>        CGEN_CPU_DESC cd;
>        bfd_vma pc;
>        disassemble_info *info;
>        char *buf;
>        int buflen;
> +      CGEN_EXTRACT_INFO *ex_info;
> +      unsigned long *insn_value;
>   {
> !   int status = (*info->read_memory_func) (pc, buf, buflen, info);
> !   if (status != 0)
> !     {
> !       (*info->memory_error_func) (status, pc, info);
> !       return -1;
> !     }
>
> !   ex_info->dis_info = info;
> !   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;
> + }
> +
> + /* Utility to print an insn.
> +    BUF is the base part of the insn, target byte order, BUFLEN bytes long.
> +    The result is the size of the insn in bytes or zero for an unknown insn
> +    or -1 if an error occurs fetching data (memory_error_func will have
> +    been called).  */
> +
> + static int
> + print_insn (cd, pc, info, buf, buflen)
> +      CGEN_CPU_DESC cd;
> +      bfd_vma pc;
> +      disassemble_info *info;
> +      char *buf;
> +      int buflen;
> + {
> +   unsigned long insn_value;
> +   const CGEN_INSN_LIST *insn_list;
> +   CGEN_EXTRACT_INFO ex_info;
> +
> +   int rc = read_insn (cd, pc, info, buf, buflen, & ex_info, & insn_value);
> +   if (rc != 0)
> +     return rc;
> +
>     /* The instructions are stored in hash lists.
>        Pick the first one and keep trying until we find the right one.  */
>
> *************** print_insn (cd, pc, info, buf, buflen)
> *** 253,258 ****
> --- 280,301 ----
>           /* Printing is handled in two passes.  The first pass parses the
>              machine insn and extracts the fields.  The second pass prints
>              them.  */
> +
> + #if CGEN_INT_INSN_P
> +         /* Make sure the entire insn is loaded into insn_value.  */
> +         if (CGEN_INSN_BITSIZE (insn) > cd->base_insn_bitsize)
> +           {
> +             unsigned long full_insn_value;
> +             int rc = read_insn (cd, pc, info, buf,
> +                                 CGEN_INSN_BITSIZE (insn) / 8,
> +                                 & ex_info, & full_insn_value);
> +             if (rc != 0)
> +               return rc;
> +             length = CGEN_EXTRACT_FN (cd, insn)
> +               (cd, insn, &ex_info, full_insn_value, &fields, pc);
> +           }
> +         else
> + #endif
>
>           length = CGEN_EXTRACT_FN (cd, insn)
>             (cd, insn, &ex_info, insn_value, &fields, pc);
> Index: opcodes/cgen-ibld.in
> ===================================================================
> RCS file: /cvs/src/src/opcodes/cgen-ibld.in,v
> retrieving revision 1.1
> diff -c -p -r1.1 cgen-ibld.in
> *** cgen-ibld.in        2000/08/04 02:21:43     1.1
> --- cgen-ibld.in        2000/08/22 19:59:17
> *************** static int extract_normal
> *** 57,62 ****
> --- 57,65 ----
>   static int extract_insn_normal
>        PARAMS ((CGEN_CPU_DESC, const CGEN_INSN *, CGEN_EXTRACT_INFO *,
>               CGEN_INSN_INT, CGEN_FIELDS *, bfd_vma));
> + static void cgen_put_insn_int_value
> +      PARAMS ((CGEN_CPU_DESC, CGEN_INSN_BYTES_PTR, int, int, CGEN_INSN_INT));
> +
>
>   /* Operand insertion.  */
>
> *************** insert_normal (cd, value, attrs, word_of
> *** 183,191 ****
> --- 186,196 ----
>     if (length == 0)
>       return NULL;
>
> + #if 0
>     if (CGEN_INT_INSN_P
>         && word_offset != 0)
>       abort ();
> + #endif
>
>     if (word_length > 32)
>       abort ();
> *************** insert_normal (cd, value, attrs, word_of
> *** 237,245 ****
>       int shift;
>
>       if (CGEN_INSN_LSB0_P)
> !       shift = (start + 1) - length;
>       else
> !       shift = word_length - (start + length);
>       *buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
>     }
>
> --- 242,250 ----
>       int shift;
>
>       if (CGEN_INSN_LSB0_P)
> !       shift = (word_offset + start + 1) - length;
>       else
> !       shift = total_length - (word_offset + start + length);
>       *buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
>     }
>
> *************** insert_insn_normal (cd, insn, fields, bu
> *** 283,289 ****
>
>   #if CGEN_INT_INSN_P
>
> !   *buffer = value;
>
>   #else
>
> --- 288,295 ----
>
>   #if CGEN_INT_INSN_P
>
> !   cgen_put_insn_int_value (cd, buffer, cd->base_insn_bitsize,
> !                          CGEN_FIELDS_BITSIZE (fields), value);
>
>   #else
>
> *************** insert_insn_normal (cd, insn, fields, bu
> *** 313,318 ****
> --- 319,341 ----
>
>     return NULL;
>   }
> +
> + /* Cover function to store an insn value into an integral insn.  Must go here
> +  because it needs <prefix>-desc.h for CGEN_INT_INSN_P.  */
> +
> + void
> + cgen_put_insn_int_value (cd, buf, length, insn_length, value)
> +      CGEN_CPU_DESC cd;
> +      CGEN_INSN_BYTES_PTR buf;
> +      int length;
> +      int insn_length;
> +      CGEN_INSN_INT value;
> + {
> +   int shift = insn_length - length;
> +   /* Written this way to avoid undefined behaviour.  */
> +   CGEN_INSN_INT mask = (((1L << (length - 1)) - 1) << 1) | 1;
> +   *buf = (*buf & ~(mask << shift)) | ((value & mask) << shift);
> + }
>
>   /* Operand extraction.  */
>
> *************** extract_normal (cd, ex_info, insn_value,
> *** 469,477 ****
> --- 492,502 ----
>         return 1;
>       }
>
> + #if 0
>     if (CGEN_INT_INSN_P
>         && word_offset != 0)
>       abort ();
> + #endif
>
>     if (word_length > 32)
>       abort ();
> *************** extract_normal (cd, ex_info, insn_value,
> *** 487,501 ****
>
>     /* Does the value reside in INSN_VALUE?  */
>
> !   if (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 >> ((start + 1) - length);
>         else
> !       value = insn_value >> (word_length - (start + length));
>         value &= mask;
>         /* sign extend? */
>         if (CGEN_BOOL_ATTR (attrs, CGEN_IFLD_SIGNED)
> --- 512,526 ----
>
>     /* Does the value reside in INSN_VALUE?  */
>
> !   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)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: CGEN: RFA: CGEN_INT_INSN_P
  2000-08-22 13:22 Dave Brolley
@ 2000-08-22 13:29 ` Dave Brolley
  2000-08-28 11:40 ` Dave Brolley
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Brolley @ 2000-08-22 13:29 UTC (permalink / raw)
  To: cgen

Dave Brolley wrote:
> 
> Hi,
> 
> I ran into a problem with the cgen-based gas/opcodes/binutils/sim
> port that I am working on. The ISA has some 16 bit insns and some
> 32 bit insns, so I set default-insn-word-bitsize=16,
> default-insn-bitsize=16 and base-insn-bitsize=16.
> 
> Now in <arch>-desc.h, CGEN_INT_INSN_P gets define to 1, since all
> of my insns are 32 bit or less. However, the current code enabled
> by CGEN_INT_INSN_P in cgen-ibld.in and cgen-dis.in does not allow
> for base-insn-bitsize != max-insn-bitsize.
> 
> 1) It aborts in 'extract_normal' because when accessing a 16 bit
> insn field at offset 16 it is called with word_offset==16. Also
> start==0, length==16, word_length==16 and total_length==32. These
> value are generated by cgen in <arch>_cgen_insert_operand and I
> believe that they are correct.
> 
> 2) Even if the abort did not occur the generated insn was wrong
> because the insns base value was not correctly onserted into the
> insn integer by insert_insn_normal.
> 
> 3) A similar problem to 2) exists in print_insn
> 
> This patch allows for CGEN_INT_INSN_P==1 with base-insn-bitsize
> != max-insn-bitsize. I have tested it against my correct work
                                                   ^^^^^^^
                                                should say
'current'
> (which has this configuration) and against the fr30 (which has
> CGEN_INT_INSN_P==0) and against another port for which
> CGEN_INT_INSN_P==1 and base-insn-bitsize == max-insn-bitsize.
> 
> OK to commit? ---- or was there a much easier way to handle
> this?  :-)
> 
> Dave
> 
>   -----------------------------------------------------------------
> 2000-08-22  Dave Brolley  <brolley@redhat.com>
> 
>         * cgen-ibld.in (cgen_put_insn_int_value): New function.
>         (insert_normal): Allow for non-zero word_offset with CGEN_INT_INSN_P.
>         (insert_insn_normal): Use cgen_put_insn_int_value with CGEN_INT_INSN_P.
>         (extract_normal): Allow for non-zero word_offset with CGEN_INT_INSN_P.
>         * cgen-dis.in (read_insn): New statis function.
>         (print_insn): Use read_insn to read the insn into the buffer and set
>         up for disassembly.
>         (print_insn): in CGEN_INT_INSN_P, make sure that the entire insn is
>         in the buffer.
> 
>   -----------------------------------------------------------------
> Index: opcodes/cgen-dis.in
> ===================================================================
> RCS file: /cvs/src/src/opcodes/cgen-dis.in,v
> retrieving revision 1.1
> diff -c -p -r1.1 cgen-dis.in
> *** cgen-dis.in 2000/08/04 02:21:43     1.1
> --- cgen-dis.in 2000/08/22 19:59:17
> *************** print_insn_normal (cd, dis_info, insn, f
> *** 187,229 ****
>       }
>   }
> 
> ! /* Utility to print an insn.
> !    BUF is the base part of the insn, target byte order, BUFLEN bytes long.
> !    The result is the size of the insn in bytes or zero for an unknown insn
> !    or -1 if an error occurs fetching data (memory_error_func will have
> !    been called).  */
> !
>   static int
> ! print_insn (cd, pc, info, buf, buflen)
>        CGEN_CPU_DESC cd;
>        bfd_vma pc;
>        disassemble_info *info;
>        char *buf;
>        int buflen;
>   {
> !   unsigned long insn_value;
> !   const CGEN_INSN_LIST *insn_list;
> !   CGEN_EXTRACT_INFO ex_info;
> 
> !   ex_info.dis_info = info;
> !   ex_info.valid = (1 << (cd->base_insn_bitsize / 8)) - 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 ();
>       }
> 
>     /* The instructions are stored in hash lists.
>        Pick the first one and keep trying until we find the right one.  */
> 
> --- 187,256 ----
>       }
>   }
> 
> ! /* Subroutine of print_insn. Reads an insn into the given buffers and updates
> !    the extract info.
> !    Returns 0 if all is well, non-zero otherwise.  */
>   static int
> ! read_insn (cd, pc, info, buf, buflen, ex_info, insn_value)
>        CGEN_CPU_DESC cd;
>        bfd_vma pc;
>        disassemble_info *info;
>        char *buf;
>        int buflen;
> +      CGEN_EXTRACT_INFO *ex_info;
> +      unsigned long *insn_value;
>   {
> !   int status = (*info->read_memory_func) (pc, buf, buflen, info);
> !   if (status != 0)
> !     {
> !       (*info->memory_error_func) (status, pc, info);
> !       return -1;
> !     }
> 
> !   ex_info->dis_info = info;
> !   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;
> + }
> +
> + /* Utility to print an insn.
> +    BUF is the base part of the insn, target byte order, BUFLEN bytes long.
> +    The result is the size of the insn in bytes or zero for an unknown insn
> +    or -1 if an error occurs fetching data (memory_error_func will have
> +    been called).  */
> +
> + static int
> + print_insn (cd, pc, info, buf, buflen)
> +      CGEN_CPU_DESC cd;
> +      bfd_vma pc;
> +      disassemble_info *info;
> +      char *buf;
> +      int buflen;
> + {
> +   unsigned long insn_value;
> +   const CGEN_INSN_LIST *insn_list;
> +   CGEN_EXTRACT_INFO ex_info;
> +
> +   int rc = read_insn (cd, pc, info, buf, buflen, & ex_info, & insn_value);
> +   if (rc != 0)
> +     return rc;
> +
>     /* The instructions are stored in hash lists.
>        Pick the first one and keep trying until we find the right one.  */
> 
> *************** print_insn (cd, pc, info, buf, buflen)
> *** 253,258 ****
> --- 280,301 ----
>           /* Printing is handled in two passes.  The first pass parses the
>              machine insn and extracts the fields.  The second pass prints
>              them.  */
> +
> + #if CGEN_INT_INSN_P
> +         /* Make sure the entire insn is loaded into insn_value.  */
> +         if (CGEN_INSN_BITSIZE (insn) > cd->base_insn_bitsize)
> +           {
> +             unsigned long full_insn_value;
> +             int rc = read_insn (cd, pc, info, buf,
> +                                 CGEN_INSN_BITSIZE (insn) / 8,
> +                                 & ex_info, & full_insn_value);
> +             if (rc != 0)
> +               return rc;
> +             length = CGEN_EXTRACT_FN (cd, insn)
> +               (cd, insn, &ex_info, full_insn_value, &fields, pc);
> +           }
> +         else
> + #endif
> 
>           length = CGEN_EXTRACT_FN (cd, insn)
>             (cd, insn, &ex_info, insn_value, &fields, pc);
> Index: opcodes/cgen-ibld.in
> ===================================================================
> RCS file: /cvs/src/src/opcodes/cgen-ibld.in,v
> retrieving revision 1.1
> diff -c -p -r1.1 cgen-ibld.in
> *** cgen-ibld.in        2000/08/04 02:21:43     1.1
> --- cgen-ibld.in        2000/08/22 19:59:17
> *************** static int extract_normal
> *** 57,62 ****
> --- 57,65 ----
>   static int extract_insn_normal
>        PARAMS ((CGEN_CPU_DESC, const CGEN_INSN *, CGEN_EXTRACT_INFO *,
>               CGEN_INSN_INT, CGEN_FIELDS *, bfd_vma));
> + static void cgen_put_insn_int_value
> +      PARAMS ((CGEN_CPU_DESC, CGEN_INSN_BYTES_PTR, int, int, CGEN_INSN_INT));
> +
> 
>   /* Operand insertion.  */
> 
> *************** insert_normal (cd, value, attrs, word_of
> *** 183,191 ****
> --- 186,196 ----
>     if (length == 0)
>       return NULL;
> 
> + #if 0
>     if (CGEN_INT_INSN_P
>         && word_offset != 0)
>       abort ();
> + #endif
> 
>     if (word_length > 32)
>       abort ();
> *************** insert_normal (cd, value, attrs, word_of
> *** 237,245 ****
>       int shift;
> 
>       if (CGEN_INSN_LSB0_P)
> !       shift = (start + 1) - length;
>       else
> !       shift = word_length - (start + length);
>       *buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
>     }
> 
> --- 242,250 ----
>       int shift;
> 
>       if (CGEN_INSN_LSB0_P)
> !       shift = (word_offset + start + 1) - length;
>       else
> !       shift = total_length - (word_offset + start + length);
>       *buffer = (*buffer & ~(mask << shift)) | ((value & mask) << shift);
>     }
> 
> *************** insert_insn_normal (cd, insn, fields, bu
> *** 283,289 ****
> 
>   #if CGEN_INT_INSN_P
> 
> !   *buffer = value;
> 
>   #else
> 
> --- 288,295 ----
> 
>   #if CGEN_INT_INSN_P
> 
> !   cgen_put_insn_int_value (cd, buffer, cd->base_insn_bitsize,
> !                          CGEN_FIELDS_BITSIZE (fields), value);
> 
>   #else
> 
> *************** insert_insn_normal (cd, insn, fields, bu
> *** 313,318 ****
> --- 319,341 ----
> 
>     return NULL;
>   }
> +
> + /* Cover function to store an insn value into an integral insn.  Must go here
> +  because it needs <prefix>-desc.h for CGEN_INT_INSN_P.  */
> +
> + void
> + cgen_put_insn_int_value (cd, buf, length, insn_length, value)
> +      CGEN_CPU_DESC cd;
> +      CGEN_INSN_BYTES_PTR buf;
> +      int length;
> +      int insn_length;
> +      CGEN_INSN_INT value;
> + {
> +   int shift = insn_length - length;
> +   /* Written this way to avoid undefined behaviour.  */
> +   CGEN_INSN_INT mask = (((1L << (length - 1)) - 1) << 1) | 1;
> +   *buf = (*buf & ~(mask << shift)) | ((value & mask) << shift);
> + }
> 
>   /* Operand extraction.  */
> 
> *************** extract_normal (cd, ex_info, insn_value,
> *** 469,477 ****
> --- 492,502 ----
>         return 1;
>       }
> 
> + #if 0
>     if (CGEN_INT_INSN_P
>         && word_offset != 0)
>       abort ();
> + #endif
> 
>     if (word_length > 32)
>       abort ();
> *************** extract_normal (cd, ex_info, insn_value,
> *** 487,501 ****
> 
>     /* Does the value reside in INSN_VALUE?  */
> 
> !   if (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 >> ((start + 1) - length);
>         else
> !       value = insn_value >> (word_length - (start + length));
>         value &= mask;
>         /* sign extend? */
>         if (CGEN_BOOL_ATTR (attrs, CGEN_IFLD_SIGNED)
> --- 512,526 ----
> 
>     /* Does the value reside in INSN_VALUE?  */
> 
> !   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)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* CGEN: RFA: CGEN_INT_INSN_P
@ 2000-08-22 13:22 Dave Brolley
  2000-08-22 13:29 ` Dave Brolley
  2000-08-28 11:40 ` Dave Brolley
  0 siblings, 2 replies; 4+ messages in thread
From: Dave Brolley @ 2000-08-22 13:22 UTC (permalink / raw)
  To: cgen

Hi,

I ran into a problem with the cgen-based gas/opcodes/binutils/sim
port that I am working on. The ISA has some 16 bit insns and some
32 bit insns, so I set default-insn-word-bitsize=16,
default-insn-bitsize=16 and base-insn-bitsize=16.

Now in <arch>-desc.h, CGEN_INT_INSN_P gets define to 1, since all
of my insns are 32 bit or less. However, the current code enabled
by CGEN_INT_INSN_P in cgen-ibld.in and cgen-dis.in does not allow
for base-insn-bitsize != max-insn-bitsize. 

1) It aborts in 'extract_normal' because when accessing a 16 bit
insn field at offset 16 it is called with word_offset==16. Also
start==0, length==16, word_length==16 and total_length==32. These
value are generated by cgen in <arch>_cgen_insert_operand and I
believe that they are correct.

2) Even if the abort did not occur the generated insn was wrong
because the insns base value was not correctly onserted into the
insn integer by insert_insn_normal.

3) A similar problem to 2) exists in print_insn

This patch allows for CGEN_INT_INSN_P==1 with base-insn-bitsize
!= max-insn-bitsize. I have tested it against my correct work
(which has this configuration) and against the fr30 (which has
CGEN_INT_INSN_P==0) and against another port for which
CGEN_INT_INSN_P==1 and base-insn-bitsize == max-insn-bitsize.

OK to commit? ---- or was there a much easier way to handle
this?  :-)

Dave
2000-08-22  Dave Brolley  <brolley@redhat.com>

	* cgen-ibld.in (cgen_put_insn_int_value): New function.
	(insert_normal): Allow for non-zero word_offset with CGEN_INT_INSN_P.
	(insert_insn_normal): Use cgen_put_insn_int_value with CGEN_INT_INSN_P.
	(extract_normal): Allow for non-zero word_offset with CGEN_INT_INSN_P.
	* cgen-dis.in (read_insn): New statis function.
	(print_insn): Use read_insn to read the insn into the buffer and set
	up for disassembly.
	(print_insn): in CGEN_INT_INSN_P, make sure that the entire insn is
	in the buffer.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2000-08-28 11:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <Pine.LNX.4.21.0008280502180.7345-100000@moshpit.cygnus.com>
2000-08-28  9:30 ` CGEN: RFA: CGEN_INT_INSN_P Dave Brolley
2000-08-22 13:22 Dave Brolley
2000-08-22 13:29 ` Dave Brolley
2000-08-28 11:40 ` Dave Brolley

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).