https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115863
--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Li Pan from comment #8)
> (In reply to Richard Biener from comment #7)
> > (In reply to Uroš Bizjak from comment #6)
> > > Please note that w/o .SAT_TRUNC the compiler is able to optimize hot loop
> > > in
> > > compress2 to:
> > >
> > > <bb 5> [local count: 536870912]:
> > > _18 = MIN_EXPR <left_8, 4294967295>;
> > > iftmp.0_11 = (unsigned int) _18;
> > > stream.avail_out = iftmp.0_11;
> > > left_37 = left_8 - _18;
> > >
> > > while .SAT_TRUNC somehow interferes with this optimization to produce:
> > >
> > > <bb 5> [local count: 536870912]:
> > > _45 = MIN_EXPR <left_8, 4294967295>;
> > > iftmp.0_11 = .SAT_TRUNC (left_8);
> > > stream.avail_out = iftmp.0_11;
> > > left_37 = left_8 - _45;
> >
> > it looks like whatever recognizes .SAT_TRUNC doesn't pay attention that
> > there are other uses of the MIN_EXPR and thus the MIN_EXPR stays live.
> >
> > IIRC :s on (match (...) is ignored (and that's good IMO) at the moment so
> > the user of the match predicate has to check.
>
> Thanks Richard.
> Yes, the .SAT_TRUNC doesn't pay any attention the other possible use of
> MIN_EXPR.
>
> As your suggestion, we may need one additional check here (like
> gimple_unsigned_sat_trunc() && no_other_MIN_EXPR_use_after_sat_trunc_p ())
> before we build the SAT_TRUNC call.
> Sorry I didn't get the point here why we need to do this, could you please
> help to explain a bit more about it? Like wrong code or something else in
> above
> sample code.
I'd amend the captured ops, like
(match (unsigned_integer_sat_trunc @0 @2)
(convert (min@2 @0 INTEGER_CST@1))
(match (unsigned_integer_sat_trunc @0 @2)
(bit_ior:c (negate (convert (gt@2 @0 INTEGER_CST@1)))
and in the transform do
if (gimple_unsigned_integer_sat_trunc (lhs, ops, NULL)
&& has_single_use (ops[1])
&& direct_internal_fn_supported_p (IFN_SAT_TRUNC,
that would be the most simple way to avoid this. Note it avoids the
alternate use for both the MIN_EXPR and the compare (for the 2nd pattern
it might be required to have more ops to check that, the convert and also the
negate). I think you can't do
(match (unsigned_integer_sat_trunc @0 @2 @3 @4)
(convert (min@2@3@4 @0 INTEGER_CST@1))
at the moment, but the machinery actually supports it for the case of
(convert?@1 (op@2 ... where @1 and @2 refer to the same object when
the compare is not there. So it might be a matter of amending parsing
here - of course it's a bit ugly.
Having a helper that can tell there's no external uses of a SSA chain
denoted by two end-points, namely the match root (the first op
you give to gimple_unsigned_integer_sat_trunc) and the "tail", the
matched ops[0] would be a nice thing to have. Note that ops[0] is
the "wrong" root, so I think we want to match the real relevant
root which would be the compare in one case and the min in the other.
>From that the requirement would be
tree op = ops[0];
while (single_imm_use (op, &use_p, &use_stmt))
{
auto def = single_ssa_def_operand (use_stmt, SSA_OP_USE);
op = DEF_FROM_PTR (def);
if (op == lhs)
return true;
}
return false;
and maybe call such helper gimple_isolated_expr_p (tree def, tree leaf);
where in principle this could support multiple leafs (just repeat the
above for each leaf).
The match.pd machinery could support this itself of course, you can
either add single_use () conditionals yourself, implement :s on the
individual operands or have a way to mark the whole expression in this
way.
So the easiest way is to do. My above remarks are for some more general
utility.
(match (unsigned_integer_sat_trunc @0)
(convert (min@2 @0 INTEGER_CST@1))
(if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type)
&& TYPE_UNSIGNED (TREE_TYPE (@0))
&& single_use (@2))
