From: Richard Biener <richard.guenther@gmail.com>
To: Yuri Rumyantsev <ysrumyan@gmail.com>
Cc: Jeff Law <law@redhat.com>, gcc-patches <gcc-patches@gcc.gnu.org>,
Igor Zamyatin <izamyatin@gmail.com>
Subject: Re: [PATCH] Simple optimization for MASK_STORE.
Date: Fri, 06 Nov 2015 12:56:00 -0000 [thread overview]
Message-ID: <CAFiYyc18GWgQKJqL=Dtq-rUSDck1prO_ho5nWftNqB_Cq90Ebg@mail.gmail.com> (raw)
In-Reply-To: <CAEoMCqR=nYnqLibLbdStqXM1WOu1cwn9mVc_Se26ALmYg_ze=g@mail.gmail.com>
On Mon, Nov 2, 2015 at 4:24 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
> Hi Richard,
>
> I've come back to this optimization and try to implement your proposal
> for comparison:
>> Btw, you didn't try the simpler alternative of
>>
>> tree type = type_for_mode (int_mode_for_mode (TYPE_MODE (vectype)));
>> build2 (EQ_EXPR, boolean_type_node,
>> build1 (VIEW_CONVERT, type, op0), build1 (VIEW_CONVERT, type, op1));
>>
>> ? That is, use the GIMPLE level equivalent of
>> (cmp (subreg:TI reg:V4SI) (subreg:TI reg:V4SI))
>
> using the following code:
>
> vectype = TREE_TYPE (mask);
> ext_mode = mode_for_size (GET_MODE_BITSIZE (TYPE_MODE (vectype)),
> MODE_INT, 0);
> ext_type = lang_hooks.types.type_for_mode (ext_mode , 1);
>
> but I've got zero type for it. Should I miss something?
Use ext_type = build_nonstandard_integer_type (GET_MODE_PRECISION
(ext_mode), 1);
Richard.
> Any help will be appreciated.
> Yuri.
>
>
> 2015-08-13 14:40 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>> On Thu, Aug 13, 2015 at 1:32 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote:
>>> Hi Richard,
>>>
>>> Did you have a chance to look at updated patch?
>>
>> Having a quick look now. Btw, you didn't try the simpler alternative of
>>
>> tree type = type_for_mode (int_mode_for_mode (TYPE_MODE (vectype)));
>> build2 (EQ_EXPR, boolean_type_node,
>> build1 (VIEW_CONVERT, type, op0), build1 (VIEW_CONVERT, type, op1));
>>
>> ? That is, use the GIMPLE level equivalent of
>>
>> (cmp (subreg:TI reg:V4SI) (subreg:TI reg:V4SI))
>>
>> ? That should be supported by the expander already, though again not sure if
>> the target(s) have compares that match this.
>>
>> Btw, the tree-cfg.c hook wasn't what was agreed on - the restriction
>> on EQ/NE_EXPR
>> is missing. Operand type equality is tested anyway.
>>
>> Why do you need to restrict forward_propagate_into_comparison_1?
>>
>> Otherwise this looks better, but can you try with the VIEW_CONVERT as well?
>>
>> Thanks,
>> Richard.
>>
>>
>>> Thanks.
>>> Yuri.
>>>
>>> 2015-08-06 14:07 GMT+03:00 Yuri Rumyantsev <ysrumyan@gmail.com>:
>>>> HI All,
>>>>
>>>> Here is updated patch which implements Richard proposal to use vector
>>>> comparison with boolean result instead of target hook. Support for it
>>>> was added to ix86_expand_branch.
>>>>
>>>> Any comments will be appreciated.
>>>>
>>>> Bootstrap and regression testing did not show any new failures.
>>>>
>>>> ChangeLog:
>>>> 2015-08-06 Yuri Rumyantsev <ysrumyan@gmail.com>
>>>>
>>>> * config/i386/i386.c (ix86_expand_branch): Implement vector
>>>> comparison with boolean result.
>>>> * config/i386/sse.md (define_expand "cbranch<mode>4): Add define
>>>> for vector comparion.
>>>> * fold-const.c (fold_relational_const): Add handling of vector
>>>> comparison with boolean result.
>>>> * params.def (PARAM_ZERO_TEST_FOR_STORE_MASK): New DEFPARAM.
>>>> * params.h (ENABLE_ZERO_TEST_FOR_STORE_MASK): new macros.
>>>> * tree-cfg.c (verify_gimple_comparison): Add test for vector
>>>> comparion with boolean result.
>>>> * tree-ssa-forwprop.c (forward_propagate_into_comparison_1): Do not
>>>> propagate vector comparion with boolean result.
>>>> * tree-vect-stmts.c (vectorizable_mask_load_store): Initialize
>>>> has_mask_store field of vect_info.
>>>> * tree-vectorizer.c: Include files ssa.h, cfghooks.h and params.h.
>>>> (is_valid_sink): New function.
>>>> (optimize_mask_stores): New function.
>>>> (vectorize_loops): Invoke optimaze_mask_stores for loops having masked
>>>> stores.
>>>> * tree-vectorizer.h (loop_vec_info): Add new has_mask_store field and
>>>> correspondent macros.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>> * gcc.target/i386/avx2-vect-mask-store-move1.c: New test.
>>>>
>>>>
>>>> 2015-07-27 11:48 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Fri, Jul 24, 2015 at 9:11 PM, Jeff Law <law@redhat.com> wrote:
>>>>>> On 07/24/2015 03:16 AM, Richard Biener wrote:
>>>>>>>>
>>>>>>>> Is there any rationale given anywhere for the transformation into
>>>>>>>> conditional expressions? ie, is there any reason why we can't have a
>>>>>>>> GIMPLE_COND where the expression is a vector condition?
>>>>>>>
>>>>>>>
>>>>>>> No rationale for equality compare which would have the semantic of
>>>>>>> having all elements equal or not equal. But you can't define a sensible
>>>>>>> ordering (that HW implements) for other compare operators and you
>>>>>>> obviously need a single boolean result, not a vector of element comparison
>>>>>>> results.
>>>>>>
>>>>>> Right. EQ/NE only as others just don't have any real meaning.
>>>>>>
>>>>>>
>>>>>>> I've already replied that I'm fine allowing ==/!= whole-vector compares.
>>>>>>> But one needs to check whether expansion does anything sensible
>>>>>>> with them (either expand to integer subreg compares or add optabs
>>>>>>> for the compares).
>>>>>>
>>>>>> Agreed, EQ/NE for whole vector compares only would be fine for me too under
>>>>>> the same conditions.
>>>>>
>>>>> Btw, you can already do this on GIMPLE by doing
>>>>>
>>>>> TImode vec_as_int = VIEW_CONVERT_EXPR <TImode> (vec_2);
>>>>> if (vec_as_int == 0)
>>>>> ...
>>>>>
>>>>> which is what the RTL will look like in the end. So not sure if making this
>>>>> higher-level in GIMPLE is good or required.
>>>>>
>>>>> Richard.
>>>>>
>>>>>> jeff
next prev parent reply other threads:[~2015-11-06 12:56 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-06 14:04 Yuri Rumyantsev
2015-05-08 9:27 ` Richard Biener
2015-05-08 18:43 ` Jeff Law
2015-05-08 19:16 ` Richard Biener
2015-05-20 14:10 ` Yuri Rumyantsev
2015-05-29 14:28 ` Yuri Rumyantsev
2015-06-09 12:15 ` Richard Biener
2015-06-18 15:41 ` Yuri Rumyantsev
2015-07-07 13:55 ` Yuri Rumyantsev
2015-07-10 5:51 ` Jeff Law
2015-07-20 15:26 ` Yuri Rumyantsev
2015-07-21 13:59 ` Richard Biener
2015-07-23 20:32 ` Jeff Law
2015-07-24 9:04 ` Yuri Rumyantsev
2015-07-24 9:24 ` Richard Biener
2015-07-24 19:26 ` Jeff Law
2015-07-27 9:04 ` Richard Biener
2015-08-06 11:07 ` Yuri Rumyantsev
2015-08-13 11:40 ` Yuri Rumyantsev
2015-08-13 11:46 ` Richard Biener
2015-11-02 15:24 ` Yuri Rumyantsev
2015-11-05 15:49 ` Yuri Rumyantsev
2015-11-06 12:56 ` Richard Biener [this message]
2015-11-06 13:29 ` Yuri Rumyantsev
2015-11-10 12:33 ` Richard Biener
2015-11-10 12:48 ` Ilya Enkovich
2015-11-10 14:46 ` Richard Biener
2015-11-10 14:56 ` Ilya Enkovich
2015-11-10 17:02 ` Mike Stump
2015-11-11 9:18 ` Richard Biener
2015-11-11 13:13 ` Yuri Rumyantsev
2015-11-12 13:59 ` Richard Biener
2015-11-19 15:20 ` Yuri Rumyantsev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAFiYyc18GWgQKJqL=Dtq-rUSDck1prO_ho5nWftNqB_Cq90Ebg@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=izamyatin@gmail.com \
--cc=law@redhat.com \
--cc=ysrumyan@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).