On Wed, Oct 23, 2024 at 8:50 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Tue, Oct 22, 2024 at 7:21 PM Andrew Pinski <quic_apin...@quicinc.com>
> wrote:
> >
> > When internal functions support was added to match
> > (r6-4979-gc9e926ce2bdc8b),
> > the check for ECF_CONST was the builtin function side. Though before
> > r15-4503-g8d6d6d537fdc,
> > there was no use of maybe_push_res_to_seq with non-const internal functions
> > so the check
> > would not make a difference.
> >
> > This adds the check for internal functions just as there is a check for
> > builtins.
> >
> > Note I didn't add a testcase because there was no non-const internal
> > function
> > which could be used on x86_64 in a decent manor.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
>
> OK.
Note there's similar correctness restrictions on the matching side, say
_1 = strlen (_2);
*_2 = '0';
_3 = strlen (_2);
_4 = _3 == _1;
if you had
(simplify
(eq (BUILT_IN_STRLEN @0) (BUILT_IN_STRLEN @0))
{ boolean_true_node; })
it would currently miscompile the above - you have to be careful when
writing patterns here and it's currently not possible to capture/compare
virtual operands in the patterns (no such in GENERIC) and genmatch
doesn't try to be clever here, but it also doesn't reject matching of
non-ECF_CONST calls since that would go too far. On the genmatch
side it might be possible to make sure everything we match uses
the same VUSE (or NULL) and has no VDEF (or when there's a VDEF
it matches the other VUSEs - which means there's at most one VDEF).
'errno' is a nuisance there - we might want to make whether a stmt
affects 'errno' a per-stmt thing so we can use range info to prune this
(IIRC there's a PR also requesting some function attribute to annotate
library functions)
Due to the 'errno' nuisance and effective disabling of some patterns
I initially refrained from doing anything about virtual operands during
matching time.
Richard.
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > PR tree-optimization/117260
> > * gimple-match-exports.cc (maybe_push_res_to_seq): Reject non-const
> > internal functions.
> >
> > Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
> > ---
> > gcc/gimple-match-exports.cc | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc
> > index d3e626a1a24..77d225825cf 100644
> > --- a/gcc/gimple-match-exports.cc
> > +++ b/gcc/gimple-match-exports.cc
> > @@ -522,6 +522,11 @@ maybe_push_res_to_seq (gimple_match_op *res_op,
> > gimple_seq *seq, tree res)
> > {
> > /* Generate the given function if we can. */
> > internal_fn ifn = as_internal_fn (fn);
> > +
> > + /* We can't and should not emit calls to non-const functions. */
> > + if (!(internal_fn_flags (ifn) & ECF_CONST))
> > + return NULL_TREE;
> > +
> > new_stmt = build_call_internal (ifn, res_op);
> > if (!new_stmt)
> > return NULL_TREE;
> > --
> > 2.43.0
> >
