This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [mips patch RFA] some objdump -M options, better reg dumps


cgd@broadcom.com writes:
> I didn't read it _really_ carefully, but your patch looks pretty good
> to me.
> 
> My one concern is about the mips_32bit_flags_p fn.  Not sure it'll
> indicate 32-bit in all the appropriate cases, because the ABI markings
> are recent...  Though i guess pre-ABI markings, you'd go by
> EF_MIPS_ARCH anyway.  Seesm reasonable I guess.

Like you say, the idea is to rely on EF_MIPS_ARCH when neither
EF_MIPS_32BITMODE or EF_MIPS_ABI is used (just like we do now).

> Also, for code clarity, personally I'd avoid the array-of-arrays and
> go for an array of struct like:
> 
> 	struct mips_mach_extentions {
> 		unsigned long extension;
> 		unsigned long base;
> 	}
> 
> then make mips_mach_extensions be defined in terms of it.

OK.

> Not sure what type is appropriate; i do know that a fair number of
> places seem to use 'unsigned long' for BFD machine type defines.
> (e.g. the disassembler, i believe.)

OK, unsigned long it is.

> > > +struct mips_arch_choice mips_arch_choices[] = {
> > > [...]
> > This looks suspiciously like a subset of the info in tc-mips.c.

[BTW, what I meant to say was, the names are a subset of the names
in tc-mips.c.]

> Yes, there's a lot of the same information but:
> 
> * some of the entries are different.  In particular, mips32 and mips64
>   disassemble the ASEs, and you _do not_ want to assemble them by
>   default IMO.

The ASE flags could be in a separate field.  They might be useful
in a shared table anyway, since gas could then do a bit more sanity
checking.

> * it has more info than the table in tc-mips.c.  (cp0 reg names, soon
>   hwr names, eventually hopefully cp2 reg names, bfd number.)

I agree that the register name stuff probably only belong in opcodes,
but you could key it off the bfd mach.  I'd still rather see the
common bits (name -> cpu/isa/bfd mach mapping) in a single place.

> * the dis one doesn't have the 'is this an isa' flag.

The flag might be useful in a common table, though.

> > Can't we use the same table for both gas and opcodes?  Not only
> > would it mean less duplicated info, it would mean consistency.
> 
> Well, we can't have that!
> 
> It's slightly worse than that, actually: Right now the table and IIRC
> also the lookup code in gas is also duplicated in gcc (w/ instructions
> to keep them in sync).

Yeah, well, that's an unfortunate side-effect of the way the src
& gcc repositories have to be kept separate.  It would be much easier
to share a table between binutils modules.

> I.e., there's already sharing, and I don't know if that sharing can be
> compatible w/ merging the table for assembly and disassembly.
> 
> These tables are useful in at least:
> 
> * gas (assembly)
> * gcc (arch selection)
> * opcodes (disassembly)
> 
> Now, the question obviously arises, "how do you do that?"  That's
> non-obvious to me especially given the fact that the table really
> would like to include bfd-related constants.

We could drop unwanted info from the table if the entries were
written using macros, much like gcc's rtl.def and tree.def.  E.g.

    ARCH ("vr4120",  3, CPU_4120, bfd_mach_mips4120, ...)
    ARCH ("sb1",    64, CPU_SB1,  bfd_mach_mips_sb1, INSN_MIPS3D | ...)

I'm not sure whether it's easier (logistically) to have
a single file shared between gcc & src or whether it's
easier to keep them separate.  But if we're going to use
a table like this in two parts of binutils, it might be
worth trying the experiment there first.

> b) i'd rather not wait for the perfect solution, to get the basic
> functionality implemented.

OK.  I'll try to do a common table "some time" next year. ;)

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]