RE: [PATCH] MIPS opcode table loads

["Moore, Catherine" <Catherine_Moore at mentor dot com>]

> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: Tuesday, November 05, 2013 2:01 PM
> To: Moore, Catherine
> Cc: binutils@sourceware.org
> Subject: Re: [PATCH] MIPS opcode table loads
> 
> First of all, sorry for the slow review.  Been snowed under with wide-int
> stuff.
> 
No problem. 

> "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.
> 
Okay, good.  I've now added these instructions to the patch.

> 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.
> 
I have a patch for the microMIPS opcode table that I'll post separately.  I've rewritten the patch to conform to suggestion #2.  It's attached.  Okay to commit?
Thanks,
Catherine

Attachment: lm.cl
Description: lm.cl

Attachment: lm.patch
Description: lm.patch