[PATCH i386 1/8] [AVX512] Adjust register classes.

[Richard Henderson]

On 08/21/2013 11:28 AM, Kirill Yukhin wrote:
>>> +	  && (mode == XImode
>>> +	      || VALID_AVX512F_REG_MODE (mode)
>>> +	      || VALID_AVX512F_SCALAR_MODE (mode)))
>>> +	return true;
>>> +
>>> +      /* In xmm16-xmm31 we can store only 512 bit modes.  */
>>> +      if (EXT_REX_SSE_REGNO_P (regno))
>>> +	return false;
>>
>> You're rejecting scalar modes here.  Not what you wanted, surely.
> Actually, I believe comment for AVX-512 part is confusing.
> We're not rejecting scalar modes, VALID_AVX512F_SCALAR_MODE allows that.
> We are rejecting all extra SSE registers, when there is no AVX-512 or
> mode is not fit for it.

Yes, I did mis-read the test.  What you have now is correct, but
I still think it could be improved.  We'll do that with followups.

>  (define_insn "*movdi_internal"
>    [(set (match_operand:DI 0 "nonimmediate_operand"
> -    "=r  ,o  ,r,r  ,r,m ,*y,*y,?*y,?m,?r ,?*Ym,*x,*x,*x,m ,?r ,?r,?*Yi,?*Ym,?*Yi")
> +    "=r  ,o  ,r,r  ,r,m ,*y,*y,?*y,?m,?r ,?*Ym,*v,*v,*v,m ,?r ,?r,?*Yi,?*Ym,?*Yi")
>  	(match_operand:DI 1 "general_operand"
> -    "riFo,riF,Z,rem,i,re,C ,*y,m  ,*y,*Yn,r   ,C ,*x,m ,*x,*Yj,*x,r   ,*Yj ,*Yn"))]
> +    "riFo,riF,Z,rem,i,re,C ,*y,m  ,*y,*Yn,r   ,C ,*v,m ,*v,*Yj,*v,r   ,*Yj ,*Yn"))]
>    "!(MEM_P (operands[0]) && MEM_P (operands[1]))"
>  {
>    switch (get_attr_type (insn))
> @@ -1896,6 +1964,8 @@
>  	  return "%vmovq\t{%1, %0|%0, %1}";
>  	case MODE_TI:
>  	  return "%vmovdqa\t{%1, %0|%0, %1}";
> +	case MODE_XI:
> +	  return "vmovdqa64\t{%g1, %g0|%g0, %g1}";
>  
>  	case MODE_V2SF:
>  	  gcc_assert (!TARGET_AVX);
> @@ -1989,7 +2059,11 @@
>       (cond [(eq_attr "alternative" "2")
>  	      (const_string "SI")
>  	    (eq_attr "alternative" "12,13")
> -	      (cond [(ior (not (match_test "TARGET_SSE2"))
> +	      (cond [(ior (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[0]))")
> +			  (and (match_test "REG_P (operands[1])")
> +			       (match_test "EXT_REX_SSE_REGNO_P (REGNO (operands[1]))")))
> +		       (const_string "XI")
> +		     (ior (not (match_test "TARGET_SSE2"))
>  			  (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
>  		       (const_string "V4SF")
>  		     (match_test "TARGET_AVX")

Better.  And while it produces the correct results, using match_operand would
be better than embedding a reference to operands within a match_test.

But since I don't want to see another 2000 line patch, I'd like you to address
this with a followup as well.

The patch is ok to commit.


r~