Re: [Patch, RTL] Eliminate redundant vec_select moves.

[Richard Sandiford <rdsandiford@googlemail.com>]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Tue, Dec 10, 2013 at 10:26 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>> On Tue, Dec 10, 2013 at 9:57 AM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>> On Tue, Dec 10, 2013 at 8:05 AM, Kirill Yukhin
>>>>> <kirill.yukhin@gmail.com> wrote:
>>>>>> On 09 Dec 14:08, H.J. Lu wrote:
>>>>>>>
>>>>>>> There are no regressions on Linux/x86-64 with -m32 and -m64.
>>>>>>> Can you check if it improves code quality on x886?
>>>>>>
>>>>>> As second thought. If Tejas and Richard are right and it is simply
>>>>>> incorrect
>>>>>> to check any offsets in this hook, may be we can end up with patch in the
>>>>>> bottom?
>>>>>
>>>>> What is wrong to pass the correct offset to
>>>>> CANNOT_CHANGE_MODE_CLASS?  Backends are free to
>>>>> ignore it.
>>>>
>>>> The point is that:
>>>>
>>>>>> - /* Vector registers do not support subreg with nonzero offsets,
>>>>>> which
>>>>>> -        are otherwise valid for integer registers.  Since we can't see
>>>>>> -        whether we have a nonzero offset from here, prohibit all
>>>>>> -         nonparadoxical subregs changing size.  */
>>>>>> -      if (GET_MODE_SIZE (to) < GET_MODE_SIZE (from))
>>>>>> -       return true;
>>>>
>>>> seems to be trying to reject things like (subreg:SF (reg:V4SF X) 1),
>>>> which is always invalid for a single-register V4SF.  See:
>>>
>>> That is correct.
>>
>> Sorry, what I mean is: that subreg is always invalid for single-
>> register V4SFs regardless of the target.  This isn't something that
>> CANNOT_CHANGE_MODE_CLASS should be expected to check.
>>
>
> Why is
>
> (define_insn "*movv4sfdi_subreg"
>  [(set (match_operand:DI 0 "nonimmediate_operand"           "=rxm")
>        (subreg:DI (match_operand:V4SF 1 "register_operand" "x") 0))]
>
> invalid?

Sorry, I don't understand.  I never said it was invalid.  I said
(subreg:SF (reg:V4SF X) 1) was invalid if (reg:V4SF X) represents
a single register.  On a little-endian target, the offset cannot be
anything other than 0 in that case.

So the CANNOT_CHANGE_MODE_CLASS code above seems to be checking for
something that is always invalid, regardless of the target.  That kind
of situation should be rejected by target-independent code instead.

In other words I'm arguing against the idea of passing the offset to
CANNOT_CHANGE_MODE_CLASS (which you seemed to be supporting in the
quote above).  I think Kirill's patch to remove the i386.c check was
the right way to go.

There's no need for a separate insn though.  Once you allow the subregs
(as per Kirill's patch), the normal move patterns will handle them.

Thanks,
Richard