Re: [PATCH] Lexra binutils

[Thiemo Seufer <ths at networkno dot de>]

Sergey Lapin wrote:
> This patch is based on Lexra patches which were submitted to binutils
> mainling list, rebased for current binutils and made it co-exist with
> normal MIPS targets.
> 
> The base revision of binutils for this patch is 2.17.
> 
> This patch should not break anything (I have tested that, so I hope),
> so it should be possible to commit it. But beforehand I'd like to have
> some advice from binutils gurus about proper usage and proper review.
> 
> Usage:
> 
> mips-linux-uclibc-as -march=lexra -mtune=lexra somefile.s
> 
> Instead of "lexra" word you could also use "lx5280" if you like.
> Theoretically, these flags should be used from gcc (as soon as I
> look into Eugeny's patch and change it appropriately).
> 
> Waiting for your feedback.
> S.
> 
> Signed-off-by: Sergey Lapin <slapin@ossfans.org>
> ---
>  bfd/bfd-in2.h         |    1 +
>  bfd/cpu-mips.c        |    4 ++-
>  gas/config/tc-mips.c  |   80 +++++++++++++++++++++++++++++++++++++++++++++++--
>  include/opcode/mips.h |    5 ++-
>  opcodes/mips-opc.c    |   25 ++++++++-------
>  5 files changed, 98 insertions(+), 17 deletions(-)
> 
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 64c022f..f8045c1 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -1757,6 +1757,7 @@ enum bfd_architecture
>  #define bfd_mach_mips16                16
>  #define bfd_mach_mips5                 5
>  #define bfd_mach_mips_sb1              12310201 /* octal 'SB', 01 */
> +#define bfd_mach_mips_lexra            0x4c58

A comment like the one for SB-1 would be nice. (Likewise for the CPU
define in mips.h.)

[snip]
> @@ -5719,10 +5726,20 @@ macro (struct mips_cl_insn *ip)
>        coproc = 1;
>        goto ld;
>      case M_LWL_AB:
> +      if (mips_opts.arch == CPU_LX5280)
> +	{
> +	  as_bad (_("opcode not supported on this processor"));
> +	  break;
> +	}
[snip: many more of those]
> @@ -8003,12 +8070,15 @@ mips_ip (char *str, struct mips_cl_insn *ip)
>        else
>  	ok = FALSE;
>  
> +      if (mips_opts.arch == CPU_LX5280 && (insn->membership & INSN_LXMISS) != 0)
> +	ok = FALSE;
> +
[snip]
> diff --git a/include/opcode/mips.h b/include/opcode/mips.h
> index 4bec5ed..f0345a2 100644
> --- a/include/opcode/mips.h
> +++ b/include/opcode/mips.h
> @@ -502,6 +502,8 @@ struct mips_opcode
>  #define INSN_5500		  0x02000000
>  /* MT ASE */
>  #define INSN_MT                   0x04000000
> +/* Absent on Lexra */
> +#define INSN_LXMISS		  0x08000000

I don't like the approach subtracting instructions from a base ISA,
IMO the flags should be strictly additive. This avoids scattering
the code with exceptions for a particular CPU, and helps the
disassembler to know about the allowed instructions.

I recommend to add a new pseudo-ISA INSN_ISA1-, consisting of the
common subset of Lexra and MIPS I instructions.

[snip]
>  /* MIPS ISA defines, use instead of hardcoding ISA level.  */
>  
> @@ -549,6 +551,7 @@ struct mips_opcode
>  #define CPU_MIPS64      64
>  #define CPU_MIPS64R2	65
>  #define CPU_SB1         12310201        /* octal 'SB', 01.  */
> +#define CPU_LX5280	0x4c58

Again, a comment for this magic number, please.

>  /* Test for membership in an ISA including chip specific ISAs.  INSN
>     is pointer to an element of the opcode table; ISA is the specified
> @@ -570,7 +573,7 @@ struct mips_opcode
>       || (cpu == CPU_VR4120 && ((insn)->membership & INSN_4120) != 0)	\
>       || (cpu == CPU_VR5400 && ((insn)->membership & INSN_5400) != 0)	\
>       || (cpu == CPU_VR5500 && ((insn)->membership & INSN_5500) != 0)	\
> -     || 0)	/* Please keep this term for easier source merging.  */
> +     || 0)	/* Please keep this term for easier source merging.  */ \

Superfluous change.


Thiemo