From: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc Patches <gcc-patches@gcc.gnu.org>,
Richard Sandiford <richard.sandiford@arm.com>
Subject: Re: [0/9] [middle-end] Add param to vec_perm_const hook to specify mode of input operand
Date: Thu, 26 May 2022 01:53:47 +0530 [thread overview]
Message-ID: <CAAgBjMmvRno=+-OecJaRYZp1f8yK_2-M0YR5Csk404Lg4WJPLg@mail.gmail.com> (raw)
In-Reply-To: <92376B2F-64EB-47F1-A2B5-2762525EF3D9@gmail.com>
On Thu, 26 May 2022 at 00:37, Richard Biener <richard.guenther@gmail.com> wrote:
>
>
>
> > Am 25.05.2022 um 21:03 schrieb Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>:
> >
> > On Wed, 25 May 2022 at 18:27, Richard Biener <richard.guenther@gmail.com> wrote:
> >>
> >>> On Tue, May 24, 2022 at 9:22 PM Prathamesh Kulkarni via Gcc-patches
> >>> <gcc-patches@gcc.gnu.org> wrote:
> >>>
> >>> On Tue, 24 May 2022 at 14:50, Richard Sandiford
> >>> <richard.sandiford@arm.com> wrote:
> >>>>
> >>>> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
> >>>>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> >>>>> index c5006afc00d..0a3c733ada9 100644
> >>>>> --- a/gcc/doc/tm.texi
> >>>>> +++ b/gcc/doc/tm.texi
> >>>>> @@ -6088,14 +6088,18 @@ for the given scalar type @var{type}. @var{is_packed} is false if the scalar
> >>>>> access using @var{type} is known to be naturally aligned.
> >>>>> @end deftypefn
> >>>>>
> >>>>> -@deftypefn {Target Hook} bool TARGET_VECTORIZE_VEC_PERM_CONST (machine_mode @var{mode}, rtx @var{output}, rtx @var{in0}, rtx @var{in1}, const vec_perm_indices @var{&sel})
> >>>>> +@deftypefn {Target Hook} bool TARGET_VECTORIZE_VEC_PERM_CONST (machine_mode @var{mode}, machine_mode @var{op_mode}, rtx @var{output}, rtx @var{in0}, rtx @var{in1}, const vec_perm_indices @var{&sel})
> >>>>> This hook is used to test whether the target can permute up to two
> >>>>> -vectors of mode @var{mode} using the permutation vector @code{sel}, and
> >>>>> -also to emit such a permutation. In the former case @var{in0}, @var{in1}
> >>>>> -and @var{out} are all null. In the latter case @var{in0} and @var{in1} are
> >>>>> -the source vectors and @var{out} is the destination vector; all three are
> >>>>> -operands of mode @var{mode}. @var{in1} is the same as @var{in0} if
> >>>>> -@var{sel} describes a permutation on one vector instead of two.
> >>>>> +vectors of mode @var{op_mode} using the permutation vector @code{sel},
> >>>>> +producing a vector of mode @var{mode}.The hook is also used to emit such
> >>>>
> >>>> Should be two spaces between “@var{mode}.” and “The”.
> >>>>
> >>>>> +a permutation.
> >>>>> +
> >>>>> +When the hook is being used to test whether the target supports a permutation,
> >>>>> +@var{in0}, @var{in1}, and @var{out} are all null.When the hook is being used
> >>>>
> >>>> Same here: missing spaces before “When”.
> >>>>
> >>>>> +to emit a permutation, @var{in0} and @var{in1} are the source vectors of mode
> >>>>> +@var{op_mode} and @var{out} is the destination vector of mode @var{mode}.
> >>>>> +@var{in1} is the same as @var{in0} if @var{sel} describes a permutation on one
> >>>>> +vector instead of two.
> >>>>>
> >>>>> Return true if the operation is possible, emitting instructions for it
> >>>>> if rtxes are provided.
> >>>>> diff --git a/gcc/match.pd b/gcc/match.pd
> >>>>> index f5efa77560c..f2a527d9c42 100644
> >>>>> --- a/gcc/match.pd
> >>>>> +++ b/gcc/match.pd
> >>>>> @@ -7596,6 +7596,8 @@ and,
> >>>>> (with
> >>>>> {
> >>>>> tree op0 = @0, op1 = @1, op2 = @2;
> >>>>> + machine_mode result_mode = TYPE_MODE (type);
> >>>>> + machine_mode op_mode = TYPE_MODE (TREE_TYPE (op0));
> >>>>>
> >>>>> /* Build a vector of integers from the tree mask. */
> >>>>> vec_perm_builder builder;
> >>>>> @@ -7703,12 +7705,12 @@ and,
> >>>>> 2-argument version. */
> >>>>> tree oldop2 = op2;
> >>>>> if (sel.ninputs () == 2
> >>>>> - || can_vec_perm_const_p (TYPE_MODE (type), sel, false))
> >>>>> + || can_vec_perm_const_p (result_mode, op_mode, sel, false))
> >>>>> op2 = vec_perm_indices_to_tree (TREE_TYPE (op2), sel);
> >>>>> else
> >>>>> {
> >>>>> vec_perm_indices sel2 (builder, 2, nelts);
> >>>>> - if (can_vec_perm_const_p (TYPE_MODE (type), sel2, false))
> >>>>> + if (can_vec_perm_const_p (result_mode, op_mode, sel2, false))
> >>>>> op2 = vec_perm_indices_to_tree (TREE_TYPE (op2), sel2);
> >>>>> else
> >>>>> /* Not directly supported with either encoding,
> >>>>
> >>>> Please replace the use of TYPE_MODE here:
> >>>>
> >>>> /* See if the permutation is performing a single element
> >>>> insert from a CONSTRUCTOR or constant and use a BIT_INSERT_EXPR
> >>>> in that case. But only if the vector mode is supported,
> >>>> otherwise this is invalid GIMPLE. */
> >>>> if (TYPE_MODE (type) != BLKmode
> >>>>
> >>>> as well.
> >>>>
> >>>> OK with those changes, thanks.
> >>> Thanks, committed the patch in ae8decf1d2b8329af59592b4fa78ee8dfab3ba5e.
> >>
> >> So the present state allows to ask can_vec_perm_const_p but the
> >> implementation asks for
> >>
> >> 431 if (direct_optab_handler (vec_perm_optab, mode) !=
> >> CODE_FOR_nothing)
> >> 432 return true;
> >>
> >> which then breaks. Also the VEC_PERMs are not yet valid. Any reason this
> >> was committed half-way through the review process of the series?
> > Hi Richard,
> > I am very sorry about that. I thought the patch was approved, and
> > committed it after testing on x86_64 and aarch64.
> > Should I revert it ?
>
> No need.
>
> > Um sorry to ask a silly question -- I am not sure why does the patch
> > break the above condition ?
> > The patch still passes mode to direct_optab_handler as before, and
> > IIUC does not affect modes
> > for vec_perm_optab, so it shouldn't affect the call to
> > direct_optab_handler above ?
>
> x86 now accepts V4SI V8SI permutes because we don’t ask it correctly and thus my naive attempt to use the new function API breaks . Not to mention the VEC_PERM IL is still rejected. I will wait for the rest of the series to be approved and pushed.
Hi,
I pushed the entire series in ae8decf1d2b8329af59592b4fa78ee8dfab3ba5e
after it was approved by Richard S.
Thanks,
Prathamesh
>
> Richard.
>
> > Thanks,
> > Prathamesh
> >>
> >> At least I have a user in the vectorizer ready - allowing more permutes
> >> from existing vectors (of different sizes now) to be SLP vectorized.
> >>
> >> Thanks,
> >> Richard.
> >>
> >>> Thanks,
> >>> Prathamesh
> >>>>
> >>>> Richard
next prev parent reply other threads:[~2022-05-25 20:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-18 7:06 Prathamesh Kulkarni
2022-05-18 9:38 ` Andre Vieira (lists)
2022-05-18 11:57 ` Richard Sandiford
2022-05-23 8:47 ` Prathamesh Kulkarni
2022-05-23 12:44 ` Richard Sandiford
2022-05-24 9:02 ` Prathamesh Kulkarni
2022-05-24 9:20 ` Richard Sandiford
2022-05-24 19:21 ` Prathamesh Kulkarni
2022-05-25 12:57 ` Richard Biener
2022-05-25 19:02 ` Prathamesh Kulkarni
2022-05-25 19:07 ` Richard Biener
2022-05-25 20:23 ` Prathamesh Kulkarni [this message]
2022-05-27 13:40 ` Richard Biener
2022-05-30 8:15 ` Richard Sandiford
2022-05-19 11:30 ` Richard Biener
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='CAAgBjMmvRno=+-OecJaRYZp1f8yK_2-M0YR5Csk404Lg4WJPLg@mail.gmail.com' \
--to=prathamesh.kulkarni@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.guenther@gmail.com \
--cc=richard.sandiford@arm.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).