include/dis-asm.h patch for cgen disassemblers

include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

Hi -

I have a scenario where I need a (cgen-based) disassembler to select
a given list of instruction sets of a multi-ISA processor, based on
a run-time condition.  This is something like the arm/thumb or
mips/mips16 split (odd PC), except that there is no trivial way to
encode the run-time condition.  The raw arch/mach parameters are
fixed, and there is no object file for the disassembler to look
into to guess from (based on PC).  So, I can either expand the
disassemble_info structure, or overload an existing field.  

I went with the latter for this patch.  It adds a new .flags option,
which tells cgen-based disassemblers to pull out the ISA bitmask out
of the .mach field.  This is no great loss, since the mach field is
not normally used for such purposes in cgen ports, and can be
overridden on a per-target basis for exceptions.

Any comments / objections?

- FChE


Index: opcodes/ChangeLog
===================================================================
@@ -1,3 +1,8 @@
+2002-01-25  Frank Ch. Eigler  <fche@redhat.com>
+
+	* cgen-dis.in (print_insn_@arch@): Support HAVE_CGEN_ISA_NOT_MACH
+	disassemble_info flag.
+

Index: opcodes/cgen-dis.in
===================================================================
@@ -380,13 +380,19 @@
 #ifdef CGEN_COMPUTE_MACH
   mach = CGEN_COMPUTE_MACH (info);
 #else
-  mach = info->mach;
+  if (info->flags & HAVE_CGEN_ISA_NOT_MACH)
+    mach = 0;
+  else
+    mach = info->mach;
 #endif
 
 #ifdef CGEN_COMPUTE_ISA
   isa = CGEN_COMPUTE_ISA (info);
 #else
-  isa = 0;
+  if (info->flags & HAVE_CGEN_ISA_NOT_MACH)
+    isa = info->mach;
+  else
+    isa = 0;
 #endif
 
   /* If we've switched cpu's, close the current table and open a new one.  */

Index: include/ChangeLog
===================================================================
@@ -1,3 +1,8 @@
+2002-01-25  Frank Ch. Eigler  <fche@redhat.com>
+
+	* dis-asm.h (HAVE_CGEN_ISA_NOT_MACH): New possible bitmask for
+	disassemble_info flags.
+

Index: include/dis-asm.h
===================================================================
@@ -93,6 +93,7 @@
      The bottom 16 bits are for the internal use of the disassembler.  */
   unsigned long flags;
 #define INSN_HAS_RELOC	0x80000000
+#define HAVE_CGEN_ISA_NOT_MACH 0x40000000 /* .mach is really a cgen isa bitmask.  */
   PTR private_data;
 
   /* Function used to get bytes to disassemble.  MEMADDR is the

include/dis-asm.h patch for cgen disassemblers

[Doug Evans]

Frank Ch. Eigler writes:
 > I have a scenario where I need a (cgen-based) disassembler to select
 > a given list of instruction sets of a multi-ISA processor, based on
 > a run-time condition.  This is something like the arm/thumb or
 > mips/mips16 split (odd PC), except that there is no trivial way to
 > encode the run-time condition.  The raw arch/mach parameters are
 > fixed, and there is no object file for the disassembler to look
 > into to guess from (based on PC).  So, I can either expand the
 > disassemble_info structure, or overload an existing field.  
 > 
 > I went with the latter for this patch.  It adds a new .flags option,
 > which tells cgen-based disassemblers to pull out the ISA bitmask out
 > of the .mach field.  This is no great loss, since the mach field is
 > not normally used for such purposes in cgen ports, and can be
 > overridden on a per-target basis for exceptions.
 > 
 > Any comments / objections?

Blech.
Down the road: "Oh.  This is a cgen port.  Maybe `mach' means `mach',
maybe it doesn't. :-("

methinks `mach' is too well established to overload.

I'd go with the first possibility, and not overload `mach'.
Instead of arch/mach in disassemble_info, have arch/mach/mumble
where mumble is up to each port.

[by "mumble" I mean "you pick the name", not "each port gets
to pick the name" ...]

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 1998 bytes --]

Hi -


dje wrote:
> [...]
> Blech.
> [...]
> I'd go with the first possibility, and not overload `mach'.
> Instead of arch/mach in disassemble_info, have arch/mach/mumble
> [...]

OK.  How about this?  It is simpler, but it furthers
disassemble_info field sprawl.


- FChE


Index: opcodes/ChangeLog
===================================================================
@@ -1,3 +1,7 @@
+2002-01-31  Frank Ch. Eigler  <fche@redhat.com>
+
+	* cgen-dis.in (print_insn_@arch@): Support disassemble_info.isas.
+

Index: opcodes/cgen-dis.in
===================================================================
@@ -386,7 +386,7 @@ print_insn_@arch@ (pc, info)
 #ifdef CGEN_COMPUTE_ISA
   isa = CGEN_COMPUTE_ISA (info);
 #else
-  isa = 0;
+  isa = info->isas;
 #endif
 
   /* If we've switched cpu's, close the current table and open a new one.  */

Index: include/ChangeLog
===================================================================
@@ -1,3 +1,8 @@
+2002-01-31  Frank Ch. Eigler  <fche@redhat.com>
+
+	* dis-asm.h (isas): New field in disassemble_info.
+	(INIT_DISASSEMBLE_INFO): Clear it.
+
Index: include/dis-asm.h
===================================================================
@@ -73,6 +73,8 @@ typedef struct disassemble_info {
   unsigned long mach;
   /* Endianness (for bi-endian cpus).  Mono-endian cpus can ignore this.  */
   enum bfd_endian endian;
+  /* A target-specific indication of applicable instruction sets.  */
+  unsigned long isas;
 
   /* Some targets need information about the current section to accurately
      display insns.  If this is NULL, the target disassembler function
@@ -271,6 +273,7 @@ extern int generic_symbol_at_address
   (INFO).flavour = bfd_target_unknown_flavour, \
   (INFO).arch = bfd_arch_unknown, \
   (INFO).mach = 0, \
+  (INFO).isas = 0, \
   (INFO).endian = BFD_ENDIAN_UNKNOWN, \
   (INFO).octets_per_byte = 1, \
   INIT_DISASSEMBLE_INFO_NO_ARCH(INFO, STREAM, FPRINTF_FUNC)


[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Doug Evans]

Frank Ch. Eigler writes:
 > > I'd go with the first possibility, and not overload `mach'.
 > > Instead of arch/mach in disassemble_info, have arch/mach/mumble
 > 
 > OK.  How about this?

I recognize wanting to have a clear name ("isas"), but methinks for
the task at hand this just repeats the problem.  The goal is (or was) to add
a target specific annotation to control disassembly.
Naming it "isas" kinda confines its usage unless one does further hackyness
as in your original patch.
[The name "isas" just seems too confining, but maybe not.
I can't think of a better anme, though methinks one is out there.
I certainly wouldn't spend much time on it.]

At any rate, IMO this patch is better than the originally proposed patch.

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Index: opcodes/ChangeLog
> ===================================================================
> @@ -1,3 +1,7 @@
> +2002-01-31  Frank Ch. Eigler  <fche@redhat.com>
> +
> +	* cgen-dis.in (print_insn_@arch@): Support disassemble_info.isas.
> +


What is the difference between an architecture, isa and machine?

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

Hi -

cagney wrote:
> What is the difference between an architecture, isa and machine?

architecture ~= bfd architecture ~= family of processors
machine ~= bfd machine ~= implementation of an architecture
isa ~= instruction set ~= group of machine instructions decodable;
                          can be a function of cpu state

- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Hi -
> 
> cagney wrote:
> 
>> What is the difference between an architecture, isa and machine?
> 
> 
> architecture ~= bfd architecture ~= family of processors


Hmm, archures.c contains:

         This enum gives the object file's CPU architecture, in a
         global sense---i.e., what processor family does it belong to?
         Another field indicates which processor within
         the family is in use.  The machine gives a number which
         distinguishes different versions of the architecture,
         containing, for example, 2 and 3 for Intel i960 KA and i960 KB,
         and 68020 and 68030 for Motorola 68020 and 68030.


> machine ~= bfd machine ~= implementation of an architecture




> isa ~= instruction set ~= group of machine instructions decodable;
>                           can be a function of cpu state


Er, ISA == Instruction Set Architecture which to me is bfd_architecture. 
  I think, here you're looking for something else.

For instance, Arm has thumb and MIPS has MIPS16.  They are modes but 
sill part of a single ISA.

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

Hi -

cagney wrote:
> [...]
> > isa ~= instruction set ~= group of machine instructions decodable;
> >                           can be a function of cpu state
> 
> Er, ISA == Instruction Set Architecture which to me is bfd_architecture. 
>   I think, here you're looking for something else.
> 
> For instance, Arm has thumb and MIPS has MIPS16.  They are modes but 
> sill part of a single ISA.

Yes, but not in an interesting sense.  It's much like the IA32 engine
inside IA64: they surely aren't the same ISA, despite being executable
by the same hardware, and operating partly on the same registers.
Sure, arm & thumb are closer together, and they may be documented in
the same publication, but that's not substantial to this question.

The conceptual issue is whether or not the choice of instructions
available is a function of processor state.  For the purposes of
tools like disassemblers and simulators, and really even assemblers
and compilers, each such group forms a separate instruction set.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Hi -
> 
> cagney wrote:
> 
>> [...]
> 
>> > isa ~= instruction set ~= group of machine instructions decodable;
>> > can be a function of cpu state
> 
>> 
>> Er, ISA == Instruction Set Architecture which to me is bfd_architecture. 
>> I think, here you're looking for something else.
>> 
>> For instance, Arm has thumb and MIPS has MIPS16.  They are modes but 
>> sill part of a single ISA.
> 
> 
> Yes, but not in an interesting sense.  It's much like the IA32 engine
> inside IA64: they surely aren't the same ISA, despite being executable
> by the same hardware, and operating partly on the same registers.
> Sure, arm & thumb are closer together, and they may be documented in
> the same publication, but that's not substantial to this question.


Yes, and both bfd_arch_ia64 and bfd_arch_i386 are defined as separate 
bfd_architectures.


> The conceptual issue is whether or not the choice of instructions
> available is a function of processor state.  For the purposes of
> tools like disassemblers and simulators, and really even assemblers
> and compilers, each such group forms a separate instruction set.


Sorry, can you try that again.

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

Hi -


dje wrote:
> [...]
> I recognize wanting to have a clear name ("isas"), but methinks for
> the task at hand this just repeats the problem.  The goal is (or was) to add
> a target specific annotation to control disassembly.
> [...]

A general hook is the "char* disassembler_options" field, which is used
as a catch-all.  Where this field is used (arm, i386, ppc), it's almost
exclusively to encode the ISA selection.  I'd prefer to formalize the
isa switch than continue to deformalize to a string option, especially
as isa switching is a correctness issue.  (Some of these disassembleres
use the string also for rendering preferences.)


> At any rate, IMO this patch is better than the originally proposed patch.

OK.  Anyone with objections?


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Doug Evans]

Frank Ch. Eigler writes:
 > A general hook is the "char* disassembler_options" field, which is used
 > as a catch-all.  Where this field is used (arm, i386, ppc), it's almost
 > exclusively to encode the ISA selection.

This is used to pass in disassembler options from the user too though,
right?  If so, it's definately not what you want here.

 > I'd prefer to formalize the
 > isa switch than continue to deformalize to a string option, especially
 > as isa switching is a correctness issue.  (Some of these disassembleres
 > use the string also for rendering preferences.)

As I said, I can't think of a better name.

No more objections here.

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Hi -
> 
> 
> dje wrote:
> 
>> [...]
>> I recognize wanting to have a clear name ("isas"), but methinks for
>> the task at hand this just repeats the problem.  The goal is (or was) to add
>> a target specific annotation to control disassembly.
>> [...]
> 
> 
> A general hook is the "char* disassembler_options" field, which is used
> as a catch-all.  Where this field is used (arm, i386, ppc), it's almost
> exclusively to encode the ISA selection.  I'd prefer to formalize the
> isa switch than continue to deformalize to a string option, especially
> as isa switching is a correctness issue.  (Some of these disassembleres
> use the string also for rendering preferences.)


I would put it the other way round.  Users use disassembler_options to 
principaly do select assembly syntax and the like.  They override the 
default mode.


>> At any rate, IMO this patch is better than the originally proposed patch.
> 
> 
> OK.  Anyone with objections?


You mean like my constant questioning?

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

Hi -

cagney wrote:

> [...]
> I would put it the other way round.  Users use disassembler_options to 
> principaly do select assembly syntax and the like.  They override the 
> default mode.

Have you looked at the opcodes/ code?  Most of the disassembler_option
substrings enable and disable groups of opcodes (instruction sets) for
decoding.  That's deeper than assembly syntax.  How an end-user performs
this picking, and why she should have to pick manually at all, are
separate issues.


> > OK.  Anyone with objections?
> You mean like my constant questioning?

Nah, those are more like cries for general elucidation.  :-)


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Hi -
> 
> cagney wrote:
> 
> 
>> [...]
>> I would put it the other way round.  Users use disassembler_options to 
>> principaly do select assembly syntax and the like.  They override the 
>> default mode.
> 
> 
> Have you looked at the opcodes/ code?  Most of the disassembler_option
> substrings enable and disable groups of opcodes (instruction sets) for
> decoding.  That's deeper than assembly syntax.  How an end-user performs
> this picking, and why she should have to pick manually at all, are
> separate issues.


GDB is only going to be changed to support a ``set disassembler-options 
<list>'' command.  To know GDB needs this change suggests someone read 
the source code.

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 2989 bytes --]

Hi -

cagney wrote:
> [...]
> >> Yes, and both bfd_arch_ia64 and bfd_arch_i386 are defined as separate 
> >> bfd_architectures.
> > 
> > That's something else, explained by other factors, such as the
> > ~12-year delay between their development, and a reluctance to
> > build any given program with both pieces.
> 
> It is also explained by someone making the rational decision that these 
> two ISA's really are different and hence deserve separate 
> bfd_architecture designations.

It is a sensible modelling decision, given that executables are
unlikely to mix ia64 and ia32 code.  The point of the example was
to show that the same piece of hardware can implement distinct
instruction sets, contrary to your possible desire to lump them
all into one mega-isa and calling them insubstantial "modes".

(BTW, I wonder whether ia64 OS kernels need to have some of each,
and if so, whether linker tricks are required to make that work.)


> Consider the PS2.  It contains a number of compute engines (for want of 
> a better term), each with their own ISA.  Do they each get their own 
> bfd_architecture or does something else happen? [...]

We did whatever must have seemed most useful at the time for their
treatment ... one bfd_arch, a bunch of bfd_mach's.  Note that all
those coprocessors have a static set of instructions.


> > Think about it.  If the processor has modes, each of which has a
> > different set of instructions available to it, then for many
> > purposes, those sets need to be formally identified as distinct
> > groups.  That's what an instruction set is, in the deeper sense.
> 
> To me thi appears to be subverting what I understand to be the original 
> intent of BFD's architecture / machine model. The subverting might be a 
> good thing, however, it needs to be carefully considered and in a 
> context that doesn't assume CGEN or SID.

Referring to BFD misses the point!  The BFD library barely cares about
individual instructions or instruction sets.  (The only example I can
come up with off the top of my head is linker relaxation -- about as
inspiring a module as gdb prologue decoding. :-)  Instruction sets as
such show up in the gas, opcodes, gcc, simulators, and so on.  Look
there for enlightenment.


> [...]
> My guess is ``environment'' (stolen from the PPC ISA manual).  For instance:
> 
>      bfd_powerpc - the ISA or ISA family
>      ppc620 - the specific implementation - supports two modes (or as 
>                                             you like to call them ISA)
>      environment - 32/64, operating/user, altivec, ...

I need a way of identifying the list of sets of instructions ("instruction
sets", get it?) valid in some current context (processor state, mode,
environment, whatever).  I think the term "isas" is better than "mode",
which is in turn better than "environment", to refer to this.  Are we
down to a mere choice-of-terminology issue?


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Consider the PS2.  It contains a number of compute engines (for want of 
>> a better term), each with their own ISA.  Do they each get their own 
>> bfd_architecture or does something else happen? [...]
> 
> 
> We did whatever must have seemed most useful at the time for their
> treatment ... one bfd_arch, a bunch of bfd_mach's.  Note that all
> those coprocessors have a static set of instructions.


I'll take that as a royal we.  I don't know what you did for the PS2 and 
will probably regret finding out.

However, please expand.

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

Hi -

cagney wrote:

> > We did whatever must have seemed most useful at the time for their
> > treatment ... one bfd_arch, a bunch of bfd_mach's.  Note that all
> > those coprocessors have a static set of instructions.
> 
> I'll take that as a royal we.  I don't know what you did for the PS2 and 
> will probably regret finding out.

It wasn't that bad.

> However, please expand.

In the context of discussing that port on a public mailing list,
and its iffy relevance to the current thread, I don't see why I
should do go into any more detail.  You know where to find the
sources.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

Hi - 

cagney wrote:
> [...]
> > Referring to BFD misses the point!  The BFD library barely cares about
> > individual instructions or instruction sets.  [...]
> 
> BFD sets the scene for its clients - LD, GDB, ...  It defines an 
> architecture / machine relationship (abet slighly broken in parts)
> and applications use that.

Yes.

> My understanding of CGEN/SID is that they turned their back on BFD and 
> defined a new set of architecture / machine relationships.

Your understanding is incorrect.

> GDB certainly doesn't want to go down that path.

No one is asking.  If the "isas" field extension goes in,
and you prefer future gdb ports to continue using only the
informal disassemble_options string, that's your prerogative.


> [...]
> > I need a way of identifying the list of sets of instructions ("instruction
> > sets", get it?) valid in some current context (processor state, mode,
> > environment, whatever).  I think the term "isas" is better than "mode",
> > which is in turn better than "environment", to refer to this.  Are we
> > down to a mere choice-of-terminology issue?
> 
> Where is the fire?
> See above, there is, I think a more fundamental problem here.  The 
> definition of bfd-architecture and machine are being quietly changed.

No.  A concept that already broadly exists outside bfd is being
formally identified in the opcodes disassemble_info structure.
Nothing in bfd is changing, and given this thread, even this lack
of change is happening exceedingly un-quietly.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> Hi -
> 
> cagney wrote:
> 
> 
>> > We did whatever must have seemed most useful at the time for their
>> > treatment ... one bfd_arch, a bunch of bfd_mach's.  Note that all
>> > those coprocessors have a static set of instructions.
> 
>> 
>> I'll take that as a royal we.  I don't know what you did for the PS2 and 
>> will probably regret finding out.
> 
> 
> It wasn't that bad.
> 
> 
>> However, please expand.
> 
> 
> In the context of discussing that port on a public mailing list,
> and its iffy relevance to the current thread, I don't see why I
> should do go into any more detail.  You know where to find the
> sources.


I chose the PS2 because its architecture is public and, I think, it very 
much illustrates the underlying issue of the current discussion.

The PS2 has a number of ISAs on a single chip.  What is the correct way 
to define this from bfd?  If you don't feel comfortable with answering 
this, then hopefully someone else will.  Off the top of my head I can 
think of several ways:

	bfd_arch_mips
		bfd_mach_mips_ps2mips
		bfd_mach_mips_ps2video

	bfd_arch_ps2
		bfd_mach_ps2mips
		bfd_mach_ps2video

	bfd_arch_mips
		bfd_mach_mips_ps2
	bfd_arch_ps2video

I think BFD to clearly document how this should be done, your isas is 
very much part of this.

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Andrew Cagney]

> I chose the PS2 because its architecture is public and, I think, it very much illustrates the underlying issue of the current discussion.
> 
> The PS2 has a number of ISAs on a single chip.  What is the correct way to define this from bfd?  If you don't feel comfortable with answering this, then hopefully someone else will.  Off the top of my head I can think of several ways:
> 
>     bfd_arch_mips
>         bfd_mach_mips_ps2mips
>         bfd_mach_mips_ps2video
> 
>     bfd_arch_ps2
>         bfd_mach_ps2mips
>         bfd_mach_ps2video
> 
>     bfd_arch_mips
>         bfd_mach_mips_ps2
>     bfd_arch_ps2video
> 
> I think BFD to clearly document how this should be done, your isas is very much part of this. 


Sorry, you did answer this.

 >> We did whatever must have seemed most useful at the time for their
 >> treatment ... one bfd_arch, a bunch of bfd_mach's.  Note that all
 >> those coprocessors have a static set of instructions.

that would be #2 above.  The question is then, is this the most 
applicable model and acceptable to BFD/BINUTILS?

It definitly means a sideways twist since ps2mips is a member of the 
bfd_arch_mips family.

Andrew

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

Hi -


cagney wrote:

> The PS2 has a number of ISAs on a single chip.  [...]

I hope you're not just yanking my chain.  You should realize that
the PS2 processor chip is more of a system-on-a-chip, with distinct
processors on one die.  The fact that they are on the same chip
is a fabrication decision, absurd to contemplate during bfd modelling.


> that would be #2 above.  The question is then, is this the most 
> applicable model and acceptable to BFD/BINUTILS? [...]

I don't really care -- it has no bearing on the current issue.

Maybe you are confusing the term "ISA" with the concept of a
"manual" or "book".  Maybe you are confusing an overall
architecture by a list of instructions that some implementation
of some architecture may execute in some modes.  Maybe you are
confusing my intent of using the "isas" field (to identify subsets
of the instruction sets implemented by the current arch/mach) with
some imagined plan to replace arch/mach.

Please get unconfused, for example by thinking about the
relationship between arm & thumb instructions.  Hint: a single
program can contain both.  Such an executable file can be
marked with a single bfd_arch/mach tag pair.  The processor
switches its interpretation of instruction memory contents
based upon internal run-time state.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

Hi -


cagney wrote:
> [...]
> What has a fabrication decision got to do with this?  The PS2 is a 
> number of different ISAs that happen to be on a single chip.

Good, we agree.

> I'm contending that this is correctly modeled within BFD as separate 
> BFD_ARCH's.  If it isn't then the documentation, at least, for BFD is wrong.
> 
> You appear to contend that it is correctly modeled as a single bfd_arch 
> and a number of different bfd_machs.  [...]

It is correct in the sense that it works (and has been working for, oh,
three years now?), it's reasonably compact in code, and it doesn't obviously
contradict any important rules.  If you prefer a new "correctness" criterion
that it fails, the it's not "correct", but the onus is on you to justify your
criterion.


> > I don't really care -- it has no bearing on the current issue.
> 
> It has direct bearing on the current issue.  Your response to the above 
> illustrates this.  

I wish you simply read what I wrote.  There is no connection
between this PS2 modelling issue and the new one.  I'll put
it in a table so you can't miss it:

          PS2                    MULTIPLE-ISA CHIP

* multiple processors        * single processor
* one instruction set per    * multiple instruction sets
  processor 
* one bfd_arch, multiple     * one bfd_arch, one bfd_mach
  bfd_machs

For the benefit of mystified binutils/cgen readers, I think the
reason cagney is so interested in the first column, is a
long-standing quixotic battle against gdb-ically incorrect
bfd modelling.  Apparently, roughly speaking, gdb's multiarch
system likes to map from bfd_arch numbers (and not bfd_arch/bfd_mach
pairs) to a vector of target-specific functions.  Using multiple
bfd_mach codes for dissimilar family members throws a monkey-wrench
into this scheme, for the simpleminded "each bfd_mach is a strict
subtype of the bfd_arch" view of the world.

Whether such a simple/rigid subtyping relationship is in fact
reasonable to insist on is a legitimate question, and I invite
Andrew to nudge the great ship bfd toward whatever heading he prefers.

In the mean time, I expect that people recognize the second
column as a distinct situation, with a distinct problem (run-time
selection among several instruction sets a la arm/thumb), for
which my patch is a reasonable solution.  


> [...]
> Who knows what you're doing.  You haven't posted any thing updating the 
> documentation and clarifying this.

AFAIK (and I at least have looked), the disassemble_info struct is not
separately documented.  I will expand on the inline comments though.


> [...] Try, instruction_subsets then?  I have strong reservations over 
> isa as it and bfd_architecture are badly overloaded.

So -- it is the "a" in "isa" that concerns you.  I will for now dodge
the issue whether it makes sense to talk about the architecture of an
instruction set per se, as opposed to the larger architecture of a
processor.  I propose to adopt the shortened "insn_sets" as the new
disassemble_info field name in question.


> Can I assume, for instance, that INSTRUCTION_SUBSETS, wouldn't be used 
> to select orthogonal ISAs that run on different compute engines?

I have no such intent -- as I've had to say too many times now, I need
the field to further refine the set of instructions identified by some
given bfd_arch/bfd_mach pair.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: include/dis-asm.h patch for cgen disassemblers

[Frank Ch. Eigler]

Hi -

cagney wrote:
> [...]
> GDB at the moment uses an arch/mach pair to select an architecture.  It 
> assumes that, just like in FSF BFD, bfd_architecture indicates a related 
> family of instruction set architectures.  [...]

That's good (except that persistent "isa" == "processor architecture"
confusion).  gdb still ranks bfd_arch above arch/mach pairs in some
contexts (e.g., gdbarch_register), but that looks changeable should
the desire arise.


> Here I consider GDB to simply be reflecting the status quo.  I don't 
> think what you've described of the PS2 is consistent with this, I don't 
> see it as my problem to nudge the great bfd ship forward.  Rather it is 
> your problem to change the the direction of the entire binutils+gdb 
> toolchain.

Dude, you're the one who brought up PS2, an irrelevant aside
to my original posting!  If gdb has no multiarch problem with
the current arrangement after all, then I have little residual
curiosity toward further discussions.  Please carry it off to
another thread.


> >> Can I assume, for instance, that INSTRUCTION_SUBSETS, wouldn't be used 
> >> to select orthogonal ISAs that run on different compute engines?
> >
> > I have no such intent -- as I've had to say too many times now, I need
> > the field to further refine the set of instructions identified by some
> > given bfd_arch/bfd_mach pair.
> 
> No, you ducked the question.  

(What, by saying "no such intent"?  I wish others "ducked questions"
with as much clarity.)

> Please clearly document this as a comment 
> that goes with the addition of this field.

Right.  Here is the version I'm about to commit.  Wording
quibbles are welcome, I guess as follow-up patches.


- FChE


Index: opcodes/ChangeLog
===================================================================
@@ -1,3 +1,7 @@
+2002-02-04  Frank Ch. Eigler  <fche@redhat.com>
+
+	* cgen-dis.in (print_insn_@arch@): Support disassemble_info.insn_sets.
+

Index: opcodes/cgen-dis.in
===================================================================
@@ -386,7 +386,7 @@
 #ifdef CGEN_COMPUTE_ISA
   isa = CGEN_COMPUTE_ISA (info);
 #else
-  isa = 0;
+  isa = info->insn_sets;
 #endif
 
   /* If we've switched cpu's, close the current table and open a new one.  */

Index: include/ChangeLog
===================================================================
+2002-02-04  Frank Ch. Eigler  <fche@redhat.com>
+
+	* dis-asm.h (disassemble_info): New field `insn_sets'.
+	(INIT_DISASSEMBLE_INFO): Clear it.
+

Index: include/dis-asm.h
===================================================================
@@ -73,6 +73,11 @@
   unsigned long mach;
   /* Endianness (for bi-endian cpus).  Mono-endian cpus can ignore this.  */
   enum bfd_endian endian;
+  /* An arch/mach-specific bitmask of selected instruction subsets, mainly
+     for processors with run-time-switchable instruction sets.  The default,
+     zero, means that there is no constraint.  CGEN-based opcodes ports
+     may use ISA_foo masks.  */
+  unsigned long insn_sets;
 
   /* Some targets need information about the current section to accurately
      display insns.  If this is NULL, the target disassembler function
@@ -271,6 +276,7 @@
   (INFO).flavour = bfd_target_unknown_flavour, \
   (INFO).arch = bfd_arch_unknown, \
   (INFO).mach = 0, \
+  (INFO).insn_sets = 0, \
   (INFO).endian = BFD_ENDIAN_UNKNOWN, \
   (INFO).octets_per_byte = 1, \
   INIT_DISASSEMBLE_INFO_NO_ARCH(INFO, STREAM, FPRINTF_FUNC)

Re: include/dis-asm.h patch for cgen disassemblers

[Doug Evans]

Frank Ch. Eigler writes:
 > Index: include/ChangeLog
 > ===================================================================
 > +2002-02-04  Frank Ch. Eigler  <fche@redhat.com>
 > +
 > +	* dis-asm.h (disassemble_info): New field `insn_sets'.
 > +	(INIT_DISASSEMBLE_INFO): Clear it.
 > +

I like "insn_sets" _much_ better than "isa".
I can see countless more disagreements avoided. :-)

Thanks.