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~
