Re: [PATCH] MIPS EVA ASE Support

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> @@ -8490,18 +8622,26 @@ macro (struct mips_cl_insn *ip)
>  
>  	  expr1.X_add_number = offset_expr.X_add_number;
>  	  normalize_address_expr (&expr1);
> -	  if (!off12 && !IS_SEXT_16BIT_NUM (expr1.X_add_number))
> +	  if (offbits != 9
> +	      && offbits != 12
> +	      && !IS_SEXT_16BIT_NUM (expr1.X_add_number))

I think this would be clearer as:

	  if ((offbits == 0 || offbits == 16)
	      && !IS_SEXT_16BIT_NUM (expr1.X_add_number))

> @@ -8510,7 +8650,7 @@ macro (struct mips_cl_insn *ip)
>  			     tempreg, tempreg, breg);
>  	      breg = tempreg;
>  	    }
> -	  if (off0)
> +	  if (offbits == 0)
>  	    {
>  	      if (offset_expr.X_add_number == 0)
>  		tempreg = breg;
> @@ -8519,20 +8659,20 @@ macro (struct mips_cl_insn *ip)
>  			     "t,r,j", tempreg, breg, BFD_RELOC_LO16);
>  	      macro_build (NULL, s, fmt, treg, tempreg);
>  	    }
> -	  else if (!off12)
> +	  else if (offbits != 9 && offbits != 12)

And "offbits == 16" here.

> @@ -10462,7 +10602,7 @@ validate_mips_insn (const struct mips_op
>  	  case 'x': USE_BITS (OP_MASK_BBITIND,	OP_SH_BBITIND);	break;
>  	  case 'X': USE_BITS (OP_MASK_BBITIND,	OP_SH_BBITIND);	break;
>  	  case 'p': USE_BITS (OP_MASK_CINSPOS,	OP_SH_CINSPOS);	break;
> -	  case 'P': USE_BITS (OP_MASK_CINSPOS,	OP_SH_CINSPOS);	break;
> +	  case 'P': USE_BITS (OP_MASK_CINSPOS,	OP_SH_CINSPOS); break;
>  	  case 'Q': USE_BITS (OP_MASK_SEQI,	OP_SH_SEQI);	break;
>  	  case 's': USE_BITS (OP_MASK_CINSLM1,	OP_SH_CINSLM1);	break;
>  	  case 'S': USE_BITS (OP_MASK_CINSLM1,	OP_SH_CINSLM1);	break;

Looks like a spurious tab->space change.

> @@ -11739,6 +11881,35 @@ mips_ip (char *str, struct mips_cl_insn 
>  		  INSERT_OPERAND (0, FZ, *ip, regno);
>  		  continue;
>  
> +		case 'j':
> +		  {
> +		    int shift = 8;
> +		    size_t i;
> +		    /* Check whether there is only a single bracketed expression
> +		       left.  If so, it must be the base register and the
> +		       constant must be zero.  */
> +		    if (*s == '(' && strchr (s + 1, '(') == 0)
> +		      continue;
> +
> +		    /* If this value won't fit into the offset, then go find
> +		       a macro that will generate a 16- or 32-bit offset code
> +		       pattern.  */
> +		    i = my_getSmallExpression (&imm_expr, imm_reloc, s);
> +		    if ((i == 0 && (imm_expr.X_op != O_constant
> +				    || imm_expr.X_add_number >= 1 << shift
> +				    || imm_expr.X_add_number < -1 << shift))
> +				    || i > 0)

Should be:

		    if ((i == 0 && (imm_expr.X_op != O_constant
				    || imm_expr.X_add_number >= 1 << shift
				    || imm_expr.X_add_number < -1 << shift))
			|| i > 0)

with the last "||" under the second "(".

> Index: gas/testsuite/gas/mips/mips.exp
> ===================================================================
> RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips.exp,v
> retrieving revision 1.222
> diff -p -u -r1.222 mips.exp
> --- gas/testsuite/gas/mips/mips.exp	14 Jun 2013 13:30:28 -0000	1.222
> +++ gas/testsuite/gas/mips/mips.exp	17 Jun 2013 20:28:48 -0000
> @@ -849,6 +849,8 @@ if { [istarget mips*-*-vxworks*] } {
>  					[mips_arch_list_matching mips64r2 \
>  					    !micromips]
>  
> +    run_dump_test_arches "eva"		[mips_arch_list_matching mips32r2 !octeon]

For the record, this is going to fail on ecoff, as Maciej pointed out
for one of the other recent patches.  I'm planning to significantly trim
the amount of testing we do on ecoff though -- hopefully within the week --
so let's commit this as-is.

OK with those changes (or non-change in the last case), thanks.

Richard