From: Jan Hubicka <hubicka@kam.mff.cuni.cz>
To: Martin Jambor <mjambor@suse.cz>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] ipa-cp: Do not create clones for values outside known value range (PR 102513)
Date: Thu, 31 Mar 2022 14:40:48 +0200 [thread overview]
Message-ID: <YkWhUHxoiqtyhfrX@kam.mff.cuni.cz> (raw)
In-Reply-To: <ri6wnhxqpmi.fsf@suse.cz>
> Hi,
>
> PR 102513 shows we emit bogus array access warnings when IPA-CP
> creates clones specialized for values which it deduces from arithmetic
> jump functions describing self-recursive calls. Those can however be
> avoided if we consult the IPA-VR information that the same pass also
> has.
>
> The patch below does that at the stage when normally values are only
> examined for profitability. It would be better not to create lattices
> describing such bogus values in the first place, however that presents
> an ordering problem, the pass currently propagates all information,
> and so both constants and VR, in no particular order when processing
> SCCs, and so this approach seemed much simpler.
>
> I plan to rearrange the pass so that it clones in multiple passes over
> the call graph (or rather the lattice dependence graph) and it feels
> natural to only do propagation for these kinds of recursion in the
> second or later passes, which would fix the issue more elegantly.
>
> Bootstrapped and tested on x86_64, OK for trunk (and perhaps also for
> GCC 11)?
Looks OK. I wonder if we can do that more genrally to avoid inlining in
such cases as well?
Honza
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2022-02-14 Martin Jambor <mjambor@suse.cz>
>
> PR ipa/102513
> * ipa-cp.cc (decide_whether_version_node): Skip scalar values
> which do not fit the known value_range.
>
> gcc/testsuite/ChangeLog:
>
> 2022-02-14 Martin Jambor <mjambor@suse.cz>
>
> PR ipa/102513
> * gcc.dg/ipa/pr102513.c: New test.
> ---
> gcc/ipa-cp.cc | 28 ++++++++++++++++++++++--
> gcc/testsuite/gcc.dg/ipa/pr102513.c | 33 +++++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr102513.c
>
> diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
> index 453e9c93cc3..ec37632d487 100644
> --- a/gcc/ipa-cp.cc
> +++ b/gcc/ipa-cp.cc
> @@ -6154,8 +6154,32 @@ decide_whether_version_node (struct cgraph_node *node)
> {
> ipcp_value<tree> *val;
> for (val = lat->values; val; val = val->next)
> - ret |= decide_about_value (node, i, -1, val, &avals,
> - &self_gen_clones);
> + {
> + /* If some values generated for self-recursive calls with
> + arithmetic jump functions fall outside of the known
> + value_range for the parameter, we can skip them. VR interface
> + supports this only for integers now. */
> + if (TREE_CODE (val->value) == INTEGER_CST
> + && !plats->m_value_range.bottom_p ()
> + && !plats->m_value_range.m_vr.contains_p (val->value))
> + {
> + /* This can happen also if a constant present in the source
> + code falls outside of the range of parameter's type, so we
> + cannot assert. */
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + {
> + fprintf (dump_file, " - skipping%s value ",
> + val->self_recursion_generated_p ()
> + ? " self_recursion_generated" : "");
> + print_ipcp_constant_value (dump_file, val->value);
> + fprintf (dump_file, " because it is outside known "
> + "value range.\n");
> + }
> + continue;
> + }
> + ret |= decide_about_value (node, i, -1, val, &avals,
> + &self_gen_clones);
> + }
> }
>
> if (!plats->aggs_bottom)
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr102513.c b/gcc/testsuite/gcc.dg/ipa/pr102513.c
> new file mode 100644
> index 00000000000..9ee5431b730
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr102513.c
> @@ -0,0 +1,33 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -Warray-bounds" } */
> +
> +extern int block2[7][256];
> +
> +static int encode_block(int block2[7][256], unsigned level)
> +{
> + int best_score = 0;
> +
> + for (unsigned x = 0; x < level; x++) {
> + int v = block2[1][x];
> + block2[level][x] = 0;
> + best_score += v * v;
> + }
> +
> + if (level > 0 && best_score > 64) {
> + int score = 0;
> +
> + score += encode_block(block2, level - 1);
> + score += encode_block(block2, level - 1);
> +
> + if (score < best_score) {
> + best_score = score;
> + }
> + }
> +
> + return best_score;
> +}
> +
> +int foo(void)
> +{
> + return encode_block(block2, 5);
> +}
> --
> 2.34.1
>
prev parent reply other threads:[~2022-03-31 12:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-14 18:17 Martin Jambor
2022-03-23 15:03 ` [PATCH PING] " Martin Jambor
2022-03-31 12:40 ` Jan Hubicka [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YkWhUHxoiqtyhfrX@kam.mff.cuni.cz \
--to=hubicka@kam.mff.cuni.cz \
--cc=gcc-patches@gcc.gnu.org \
--cc=mjambor@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).