optimisation for default_print_insn().

optimisation for default_print_insn().

[matthew green]

hi folks.


i've been looking at the cgen generated disassemblers lately
and i've noticed that it reads target memory twice for the
initial portion of the instruction.  in default_print_insn(),
there is a call to (*info->read_memory_func)() and then it
calls print_insn().  the first thing print_insn() does is
call read_insn() which performs the exact same read_memory_func
call (same arguments, buffers, etc.)  as read_insn() is used 
in other places, i believe the best way to fix this double
read is to remove the copy in default_print_insn().  i have
tested this with a couple of ports and nothing appeas to be
broken.


the patch below simply comments the block in
default_print_insn() rather than removing it entirely.  if
people think it should be removed, i can change my patch.


OK to commit?


.mrg.


2001-04-09  matthew green  <mrg@redhat.com>

	* cgen-dis.in (default_print_insn): Remove redundant call to
	(*info->read_memory_func)().


Index: cgen-dis.in
===================================================================
RCS file: /cvs/src/src/opcodes/cgen-dis.in,v
retrieving revision 1.5
diff -p -r1.5 cgen-dis.in
*** cgen-dis.in	2001/01/09 17:00:21	1.5
--- cgen-dis.in	2001/04/09 10:55:03
*************** default_print_insn (cd, pc, info)
*** 319,324 ****
--- 319,325 ----
    char buf[CGEN_MAX_INSN_SIZE];
    int status;
  
+ #if 0	/* Done in print_insn().  */
    /* Read the base part of the insn.  */
  
    status = (*info->read_memory_func) (pc, buf, cd->base_insn_bitsize / 8, info);
*************** default_print_insn (cd, pc, info)
*** 327,332 ****
--- 328,334 ----
        (*info->memory_error_func) (status, pc, info);
        return -1;
      }
+ #endif
  
    return print_insn (cd, pc, info, buf, cd->base_insn_bitsize / 8);
  }

optimisation for default_print_insn().

[Doug Evans]

matthew green writes:
 > i've been looking at the cgen generated disassemblers lately
 > and i've noticed that it reads target memory twice for the
 > initial portion of the instruction.  in default_print_insn(),
 > there is a call to (*info->read_memory_func)() and then it
 > calls print_insn().  the first thing print_insn() does is
 > call read_insn() which performs the exact same read_memory_func
 > call (same arguments, buffers, etc.)  as read_insn() is used 
 > in other places, i believe the best way to fix this double
 > read is to remove the copy in default_print_insn().  i have
 > tested this with a couple of ports and nothing appeas to be
 > broken.
 > 
 > 
 > the patch below simply comments the block in
 > default_print_insn() rather than removing it entirely.  if
 > people think it should be removed, i can change my patch.

Assuming your patch goes in, don't use #if 0's.
They just reduce the signal/noise ratio.
[sometimes they're ok, but not in this case]

 > OK to commit?

I'd rather not.  Studying the code, the comment above print_insn,
and the m32r port makes it seem to me that the read_insn
at the start of print_insn is the duplicate.
Why was it added?

Re: optimisation for default_print_insn().

[Frank Ch. Eigler]

Hi -

:  > OK to commit?
: 
: I'd rather not.  Studying the code, the comment above print_insn,
: and the m32r port makes it seem to me that the read_insn
: at the start of print_insn is the duplicate.
: Why was it added?

It looks related to fr30 assembly support; one of the first ports
that exercised varying length instructions.  To me, it makes sense
to have the read_insn() call in print_insn().  The ambiguity could
be resolved by eliminating that buf[] argument that passes between
default_print_insn and print_insn.  It seems that the only reason
for having two calls to read_insn() in the first place was the hack
to decode the 16-bit-insn-pair vs. single-32-bit-insn alternatives
in the m32r disassembler.

- FChE