From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Brolley To: cgen@sources.redhat.com Subject: Re: CGEN: RFA: CGEN_INT_INSN_P Date: Tue, 22 Aug 2000 13:29:00 -0000 Message-id: <39A2E2FE.72D22DA7@redhat.com> References: <39A2E15A.B3BDF453@redhat.com> X-SW-Source: 2000-q3/msg00051.html 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 -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 _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 > > * 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 -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)