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]

Re: PATCH: More mips3264 support


I'm not Nick, but i'll comment anyway!  8-)

echristo@redhat.com ("Eric Christopher") writes:
> 	* mips-opc.c: Add support for dmfc1, dmtc1, dmtc2, additional modes
> 	for mfc1 and mtc1.

I disagree with these.  more on these at the end.


> 	* mips-dis.c: Add support for bfd_mach_mipsisa32 and
> 	bfd_mach_mipsisa64.

puzzled about these (and the related changes elsewhere that use them).
"MIPS32 and MIPS64 ISA" is what the existing bfd_mach_mips5 and
bfd_mach_mips64 were meant to address.

Why are new constants needed?  Am I missing something Subtle here?

(mips64isa is a name in mips_cpu_info_table in gas only because
historically mips64 is taken.)

As target names go, I think i'm fine with mipsisa64, though, so stuff
related to adding that is OK by me.


> src/ld/ChangeLog
> 2001-07-19  Eric Christopher  <echristo@redhat.com>
> 	    Jason Eckhardt  <jle@redhat.com>
> 
> 	* ldmain.c (get_emulation): Add support for -mips32 and -mips64.

perhaps silly, but the order in which you've sorted the options seems
... wrong no matter how you look at it.  8-)

(looks good to minimize diffs & conflicts though, good for your own
tree maintenance.  8-)


> src/bfd/ChangeLog
> 2001-07-19  Eric Christopher  <echristo@redhat.com>
> 	    Jason Eckhardt  <jle@redhat.com>
> 
> 	* bfd/archures.c: Add mipsisa32 and mipsisa64.
> 	* bfd/cpu-mips.c: Ditto.
> 	* bfd/elf32-mips.c (_bfd_mips_elf_final_write_processing): Ditto.

same issue as above.


> src/gas/ChangeLog
> 2001-07-19  Eric Christopher  <echristo@redhat.com>
> 	    Jason Eckhardt  <jle@redhat.com>
> 
> 	* config/tc-mips.c (mips_cpu_info): Add support for mipsisa32,
> 	5kc, and 20kc.

if you're going to configure as mipsisaNN-whatever, don't you also
need support in here for mipsisa64?  ("mips64isa" was an attempt to do
that, but I can imagine it'll run afoul of more autoconf lossage than
will mipsisa64...  if you agree, you might also nuke mips64isa.)

for the MIPS32-4K cores, when reformulating this table I made it:

  { "MIPS32-4K",      0,      ISA_MIPS32,     CPU_MIPS32_4K, },
  { "4kc",            0,      ISA_MIPS32,     CPU_MIPS32_4K, },
  { "4km",            0,      ISA_MIPS32,     CPU_MIPS32_4K, },
  { "4kp",            0,      ISA_MIPS32,     CPU_MIPS32_4K, },
  { "mips32-4kc",     0,      ISA_MIPS32,     CPU_MIPS32_4K, },
  { "mips32-4km",     0,      ISA_MIPS32,     CPU_MIPS32_4K, },
  { "mips32-4kp",     0,      ISA_MIPS32,     CPU_MIPS32_4K, },
 
the notion being:

	* canonical name first (upper case, though it's compared
	insensitively when searching)

	* variants that the user might resonably type, afterward.

You might want to do the same for the 5k and 20k I don't really care.
If you do want, the entries from my (not yet submitted -- you beat me
8-) source tree are:

  /* MIPS64 5K CPU */
  { "MIPS64-5K",        0,      ISA_MIPS64,     CPU_MIPS64_5K, },
  { "5kc",              0,      ISA_MIPS64,     CPU_MIPS64_5K, },
  { "mips64-5kc",       0,      ISA_MIPS64,     CPU_MIPS64_5K, },
  
  /* MIPS64 20K CPU */  
  { "MIPS64-20K",       0,      ISA_MIPS64,     CPU_MIPS64_20K, },
  { "20kc",             0,      ISA_MIPS64,     CPU_MIPS64_20K, },
  { "mips64-20kc",      0,      ISA_MIPS64,     CPU_MIPS64_20K, },


Your current entries are a bit bogus: the table is search w/o case
sensitivity, so there's no point in having entries that differ only by
case.



Any reason for adding the blank line in opcode/mips.h?  8-)



and now back to the mips-opc.c changes:

> Index: opcodes/mips-opc.c

> +{"dmfc1",   "t,S,H",    0x44200000, 0xffe007f8, LCD|WR_t|RD_S|FP_S,     I64},

If you look in the MIPS32/MIPS64 manuals (e.g.
http://www.mips.com/declassified/Declassified_2001/MD00087-2B-MIPS64BIS-AFP-00.95.pdf),
they _do not_ define as 'sel' code for {d,}m[ft]c1!

I believe this is intentional and "correct": the FP architecture has
the FP regs, but a selector within them has no well-defined meaning.

if you look at the descriptions of all of the mfc/mtc opcodes, you'll
see that, alone, 1 is missing 'sel'.  (of course, you won't see c3 at
all.  8-)


> +{"dmfc2",   "t,S,H",    0x48200000, 0xffe007f8, LCD|WR_t|RD_S|FP_S,     I64},

This one I Just Don't Get, and I have three indepent issues with this
set of changes as is:

* You're adding interpretation of $fN as the cp register arg for c2?
Does that make sense?  It's not the FP coprocessor.  If i _is_ an FP
coprocessor on some machines, well, I'd say that that encoding should
be machine-specific.  but just saying e.g. "dmfc2 $10, $f3, 0" for
most MIPS processors doesn't make sense to me.

* you added it for dmfc2 and dmtc2, but not for mfc2 and mtc2.  That
seems incorrect.

* you added "t,S,H" variants, but didn't add "t,S" variants.  If the
former is appropriate, the latter must be as well I'd _think_!


chris


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