On Tue, 2 Feb 2021, Jakub Jelinek wrote:
> On Tue, Feb 02, 2021 at 11:06:33AM +0100, Richard Biener wrote:
> > So I fear this only covers parts of the paths simplifications can
> > end up used. Now one question is whether we want to allow
> > "invalid" intermediate transforms if a final match makes it
> > "valid" again. If not then doing the verification in
> > gimple-match.c itself might be preferable. OTOH I think that
> > most patterns should verify themselves - it's also odd that
> > v << 1 >> 1 is supported but not v & vec-cst - we might want to
> > require some basic support from targets. I realize the PR is
> > one of the odd V1mode cases which we might want to support better
> > during RTL expansion (or require targets to be more sane here).
> >
> > So in the end I don't like the patch much, not at this stage
> > anyway.
>
> I'm afraid we can't just ignore it for GCC 11, there are many passes after
> veclower and we have hundreds of folders on vector ops and many of them can
> just turn something that is handled into something that isn't, even with
> normal vector types and not the corner cases. Not all targets support all
> optabs...
> What about a modification of the patch which instead of preventing the
> simplifications clears PROP_gimple_lvec property and schedule another
> veclower pass right before isel which would be only run if the property
> isn't set (I think the -O0 version of the pass already works that way).
I don't say ignore the problem. All I say is that the x86 target
should either not advertise V1DF shifts or advertise the basic
ops that reasonable simplification would expect to exist.
The testcases sofar are artificially generating V1DF operations.
Now, if we want to have any fully automatic way of disabling
transforms like you propose then IMHO it should be based on
common a "is this op supported" machinery shared by vector lowering
and the new machinery (and eventually also used by RTL expansion
and the optab checks that exist for the vectorizers purpose).
Otherwise it's just a quite fragile (and invisble) machinery again.
Btw, I just can find V1DI mentioned in mmx.md but I can't find
rotate or shift patterns that would match?
That said, for match.pd result verification IMHO the correct
place is a gimple_match_op::target_supported_p routine or so that we'd
call
in gimple-match.c like
if (__builtin_expect (dump_file && (dump_flags & TDF_FOLDING), 0))
fprintf (dump_file, "Applying pattern %s:%d, %s:%d\n", "match.pd", 600,
__FILE__, __LINE__);
{
res_op->set_op (NOP_EXPR, type, 1);
{
tree _o1[2], _r1;
_o1[0] = captures[0];
_o1[1] = captures[1];
gimple_match_op tem_op (res_op->cond.any_else (),
TRUNC_MOD_EXPR, TREE_TYPE (_o1[0]), _o1[0], _o1[1]);
tem_op.resimplify (lseq, valueize);
<---- here
_r1 = maybe_push_res_to_seq (&tem_op, lseq);
vector lowering could then build a gimple_match_op and just
call the new routine as predicate. We could return a tri-state
to explicitely signal "don't know" (aka not fully implement the
function but try to be spot-on for vector optabs).
That said, I'd prefer a x86 specific solution for the PR at
this point (just kill those shift V1DImode optab entries?)
Richard.
