This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, RTL] Eliminate redundant vec_select moves.
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Kirill Yukhin <kirill dot yukhin at gmail dot com>, Richard Henderson <rth at redhat dot com>, Tejas Belagod <tbelagod at arm dot com>, "Yukhin, Kirill" <kirill dot yukhin at intel dot com>, Jeff Law <law at redhat dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Uros Bizjak <ubizjak at gmail dot com>, Jakub Jelinek <jakub at redhat dot com>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Tue, 10 Dec 2013 10:46:39 -0800
- Subject: Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Authentication-results: sourceware.org; auth=none
- References: <529F666F dot 4000507 at redhat dot com> <CAMe9rOo+2LnE=T9y7bmoxfWov+T4WDizTmpU5jFhpYe_xadgXA at mail dot gmail dot com> <52A07CF6 dot 6010003 at arm dot com> <CAMe9rOpZ41Qe-PqoqyJaVaYPSQfQXSkXPJeUQa23v2=0UabSXA at mail dot gmail dot com> <20131205134000 dot GG44339 at msticlxl57 dot ims dot intel dot com> <20131209064909 dot GA21317 at msticlxl57 dot ims dot intel dot com> <52A593B1 dot 6080406 at arm dot com> <CAMe9rOod87YRhu5vYfHUvDEtG_7_VJHafmUUGc=2Sj9q92SAtQ at mail dot gmail dot com> <CAMe9rOrbyJku55xx0RNFaathvRPSJXwZ5g6ad5v9q+NGPdg9tg at mail dot gmail dot com> <CAMe9rOoCz-9QM8-zMsPkxKnzJ2=M8D9LYKuRFAjwKKP4EU4acg at mail dot gmail dot com> <20131210160532 dot GB25880 at msticlxl57 dot ims dot intel dot com> <CAMe9rOo2FtK7Xk1-f__UMgxS7Q8reG8on2MxboiLGMQDSO64Mg at mail dot gmail dot com> <87bo0o7fn3 dot fsf at talisman dot default> <CAMe9rOr3QBQT6EcvthtgAKFFoFb9YpJpZLZZ_3Wrmxy1UURHeQ at mail dot gmail dot com> <8738m07eaj dot fsf at talisman dot default> <CAMe9rOq1hCZgrzHBjAdv6RUk3PJa_9JS_nz0xXaya16L11M-2w at mail dot gmail dot com> <87y53s5yvr dot fsf at talisman dot default>
On Tue, Dec 10, 2013 at 10:44 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> "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.
>
We will wait for Kirill's results.
--
H.J.
- References:
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.
- Re: [Patch, RTL] Eliminate redundant vec_select moves.