On Thu, Apr 21, 2016 at 3:54 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Apr 21, 2016 at 6:48 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> On Thu, Apr 21, 2016 at 3:43 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>> On Thu, Apr 21, 2016 at 6:33 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>>> On Thu, Apr 21, 2016 at 2:59 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>
>>>>>> We know, that const_int (-1) is allowed with TARGET_SSE2 and that
>>>>>> const_wide_int (-1) is allowed with TARGET_AVX2. Probably we don't
>>>>>> have to check AVX512F in standard_sse_constant_p, as it implies
>>>>>> TARGET_AVX2.
>>>>>>
>>>>>> As said, it is the job of insn mode attributes to emit correct
>>>>>> instruction.
>>>>>>
>>>>>> Based on the above observations, mode checks for -1 are not needed in
>>>>>> standard_sse_constant_p.
>>>>>
>>>>> void
>>>>> ix86_expand_vector_move (machine_mode mode, rtx operands[])
>>>>> {
>>>>> rtx op0 = operands[0], op1 = operands[1];
>>>>> /* Use GET_MODE_BITSIZE instead of GET_MODE_ALIGNMENT for IA MCU
>>>>> psABI since the biggest alignment is 4 byte for IA MCU psABI. */
>>>>> unsigned int align = (TARGET_IAMCU
>>>>> ? GET_MODE_BITSIZE (mode)
>>>>> : GET_MODE_ALIGNMENT (mode));
>>>>>
>>>>> if (push_operand (op0, VOIDmode))
>>>>> op0 = emit_move_resolve_push (mode, op0);
>>>>>
>>>>> /* Force constants other than zero into memory. We do not know how
>>>>> the instructions used to build constants modify the upper 64 bits
>>>>> of the register, once we have that information we may be able
>>>>> to handle some of them more efficiently. */
>>>>> if (can_create_pseudo_p ()
>>>>> && register_operand (op0, mode)
>>>>> && (CONSTANT_P (op1)
>>>>> || (SUBREG_P (op1)
>>>>> && CONSTANT_P (SUBREG_REG (op1))))
>>>>> && !standard_sse_constant_p (op1))
>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>
>>>>> What should it return for op1 == (VOIDmode) -1 when
>>>>> TARGET_AVX is true and TARGET_AVX2 is false for
>>>>> mode == TImode and mode == OImode?
>>>>>
>>>>> op1 = validize_mem (force_const_mem (mode, op1));
>>>>
>>>> Let me rethink and redesign this whole mess, so we will have
>>>> consistent predicates.
>>>
>>> The problem is because -1 has no mode. We can't tell
>>> if -1 is a valid SSE constant without mode. That is my
>>> change to standard_sse_constant_p and
>>> ix86_expand_vector_move is for. It is sufficient for
>>> all my tests, including benchmark runs.
>>
>> I'm not against mode checks, but IMO, we have to do these checks in
>> predicates, where we know operand mode.
>
> I tried and it doesn't work since the correct mode may not be always
> available in predicates. Yes, they pass mode. But they just do
>
> mode = GET_MODE (op);
>
> which returns VOIDmode for -1.
Well, looking at generated gcc/insns-preds.c, the predicates do:
(mode == VOIDmode || GET_MODE (op) == mode).
They *check* and don't *assign* "mode" variable.
So, I see no problem checking "mode" variable (that gets the value
from the pattern) in the predicates.
Uros.
