On Wed, 25 May 2022 at 18:27, Richard Biener <richard.guent...@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.sandif...@arm.com> wrote:
> > >
> > > Prathamesh Kulkarni <prathamesh.kulka...@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 ?
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 ?
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
