Re: [PATCH] PR tree-optimization/105668: Provide RTL expansion for VEC_COND_EXPR.

[Richard Biener <richard.guenther@gmail.com>]

On Mon, May 23, 2022 at 8:44 AM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This resolves PR tree-optimization/105668, a P1 ice-on-valid regression
> triggered by my recent patch to add a vec_cmpeqv1tiv1ti define_expand
> to the i386 backend.  The existence of this optab currently leads GCC
> to incorrectly assume the existence of a corresponding vcond_mask for
> V1TImode.
>
> I believe the best solution (of the three possible fixes) is to allow
> gimple_expand_vec_cond_expr to fail (return NULL) when a suitable optab
> to generate a IFN_VCOND_MASK isn't available, but instead allow RTL
> expansion to provide a default implementation using vector mode logic
> operations.  On x86_64, the equivalent of a pblend can be generated in
> three instructions using pand, pandn and pxor.  In fact, this fallback
> implementation is already used in ix86_expand_sse_movcc when the -march
> doesn't provide a suitable instruction.  This patch provides that
> functionality to all targets in the middle-end, allowing the vectorizer(s)
> to safely assume support for VEC_COND_EXPR (when the target has suitable
> vector logic instructions).
>
> I should point out (for the record) that the new expand_vec_cond_expr
> function in expr.cc is very different from the function of the same name
> removed by Matin Liska in June 2020.
> https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547097.html
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=502d63b6d6141597bb18fd23c8
> 7736a1b384cf8f
> That function simply expanded the vcond_mask optab and failed if it
> wasn't available, which is currently the task of the gimple-isel pass.
> The implementation here is a traditional RTL expander, sythesizing the
> desired vector conditional move using bit-wise XOR and AND instructions
> of the mask vector.
>
> At some point in the future, gimple-isel could be enhanced to consider
> alternative vector modes, as a V1TI blend/vec_cond_expr may be implemented
> using V2DI, V4SI, V8HI or V16QI.  Alas, I couldn't figure out how to
> conveniently iterate over the desired modes, so this enhancement is left
> for a follow-up patch.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32} with
> no new failures.  Ok for mainline?

No, first of all the purpose of ISEL is to get rid of _all_ VEC_COND_EXPRs.
So iff then this fallback would have to reside in the ISEL pass, replacing the
GIMPLE with target supported GIMPLE.

But then it is the task of tree-vect-generic.cc to turn not target supported
GIMPLE into target supported GIMPLE - and the issue in the PR in question
is that at its point we basically have _1 < _2 ? _3 : _4 which _is_ supported
by the target but passes inbetween vector lowering and ISEL hide
_1 < _2 via a PHI node and so the GIMPLE is no longer target supported.

That would be the thing to fix - I'll note we put us into the corner of
needing to keep the SSA def of the VEC_COND_EXPR condition
"next" (as in SSA def) to the VEC_COND_EXPR, something that's
difficult to maintain, especially when so man passes run inbetween.

So the solution might be to somehow move the two closer together,
maybe as much as merging the VEC_COND_EXPR part into
vector lowering itself (with the disadvantage of more difficult
to deal with IL).  Or alternatively have vectors lowered earlier
for those produced by user code and have "final" lowering done
as part of RTL expansion (so in ISEL then).

Richard.

>
> 2022-05-23  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         PR tree-optimization/105668
>         * expr.cc (expand_vec_cond_expr): New function to expand
>         VEC_COND_EXPR using vector mode logical instructions.
>         (expand_expr_real_2) <case VEC_COND_EXPR>: Call the above.
>         * gimple-isel.cc (gimple_expand_vec_cond_expr): Instead of
>         asserting, retun NULL when the target's get_vcond_mask_icode
>         returns CODE_FOR_nothing.
>
> gcc/testsuite/ChangeLog
>         PR tree-optimization/105668
>         * gcc.target/i386/pr105668.c: New test case.
>
> Roger
> --
>