This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS opcode table loads
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Moore\, Catherine" <Catherine_Moore at mentor dot com>
- Cc: "binutils\ at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 05 Nov 2013 19:00:52 +0000
- Subject: Re: [PATCH] MIPS opcode table loads
- Authentication-results: sourceware.org; auth=none
- References: <FD3DCEAC5B03E9408544A1E416F11242012EA8D017 at NA-MBX-04 dot mgc dot mentorg dot com>
First of all, sorry for the slow review. Been snowed under with
wide-int stuff.
"Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> This is the first patch for the PMC errata that we discussed on the gcc
> list. It changes the INSN_LOAD_MEMORY_DELAY pinfo bit to
> INSN_LOAD_GPR. Tested without regressions for MIPS ELF and MIPS Linux.
> It also adds INSN_LOAD_GPR to any load instruction.
>
> I may have missed a load instruction, but hopefully I found them all.
> Also, I hope that the new ones that I marked are valid as well.
Yeah, the list looks good to me. The only missing ones I could see
were the XLR instructions LDADDW, LDADDWU, LDADDD, SWAPW, SWAPWU and SWAPD,
all of which are read-modify-write. If we find more we can add them
as-and-when.
The patch adds the new flag to coprocessor as well as GPR loads,
so the name seems a bit misleading. I think we should either
(a) restrict it to GPR loads and continue to use INSN_COPRO_MEMORY_DELAY
for coprocessors or
(b) rename it to INSN_LOAD_MEMORY and include it in CLD:
#define CLD (INSN_LOAD_MEMORY|INSN_COPROC_MEMORY_DELAY)
I've a slight preference for (2) because of the symmetry with
INSN_STORE_MEMORY. The flag would then be LM, for consistency with SM.
At some point we should also add it to the MIPS16 and microMIPS tables,
but that can wait.
Thanks,
Richard