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

[cgd at broadcom dot com]

At 20 Dec 2002 09:13:04 +0000, Richard Sandiford wrote:
> cgd@broadcom.com writes:
> > wanted the ability to disable symbolic names for GPRs,
> 
> Bless you. ;)

heh.

so you "enjoy" tweaking register names in testsuite entries, too, and
"enjoy" worrying about how they'll break when run w/ an assembler w/ a
different default ABI?  8-)


> > * MIPS VR4111, MIPS VR4120, MIPS VR5400, and MIPS VR5500 tests are
> >   failing for ECOFF now, since objdump opts for those test didn't
> >   include the arch specs.  (generally better for MIPS bits to test
> >   disassembly w/ explicit arch flags, and test object file markings on
> >   a per object file format basis with a separate test.  I'll fix these
> >   at some point.)
> 
> FWIW, the patch I sent here (unreviewed, nudge, nudge ;)
> 
>   http://sources.redhat.com/ml/binutils/2002-12/msg00280.html
> 
> could easily have some extra "good_combination" lines so that
> it tests all the supported members of the VR series.  If that
> patch goes in, I'll add explicit architecture names to the
> gas testsuite (should qualify as "obvious").

Your testsuite entry there is neat.  It doesn't directly check that
the assembler produces the right bits, but does make sure the right
thing happens after link.  (My point above was really about checks for
the assembler producing the right thing.)

Probably be good to use a similar mechanism in the gas assembler tests
to check that the assembler is doign the right thing.  let us replace
bunch of the elf e_flags tests...

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.

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.

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

I didn't know libiberty provided ARRAY_SIZE, that's handy, i think
i'll use that.  8-)


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

Looks != is.  8-)

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.

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

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


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

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.

(I don't know if there are any special defines defined during the gcc
build vs. during gas/opcodes builds...  if so, we could conditionalize
out the use of the BFD names since gcc doesn't rely on BFD...)


So in summary, I think this is a good idea, but:

a) i'm not really sure how to go about it, and

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

If you can tell me how to handle (a), i'd be glad to look further.


> E.g. with the patch it stands, it looks like "4000" isn't accepted
> as a synonym for "r4000" by opcodes but it is by gas.

"Is this a bug?"  8-)

I think accepting unadorned "4000" is historical in nature an
annoying.

Don't even _think_ about "4k".  8-)



cgd