This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: PATCH: More mips3264 support
- To: echristo at redhat dot com
- Subject: Re: PATCH: More mips3264 support
- From: cgd at sibyte dot com
- Date: 22 Jul 2001 11:11:40 -0700
- cc: binutils at sources dot redhat dot com
- References: <995717531.23476.24.camel@ghostwheel.cygnus.com><mailpost.995714090.17770@postal.sibyte.com>
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