On 6/2/21 3:29 AM, Richard Biener wrote:
On Tue, Jun 1, 2021 at 4:24 PM Andrew MacLeod <amacl...@redhat.com> wrote:
On 6/1/21 3:34 AM, Richard Biener wrote:
On Tue, Jun 1, 2021 at 3:38 AM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
An ongoing issue is the the order we evaluate things in can affect
decisions along the way. As ranger isn't a fully iterative pass, we can
sometimes come up with different results if back edges are processed in
different orders.
One of the ways this can happen is when the cache is propagating
on-entry values for an SSA_NAME. It calculates outgoing edge values and
the gori-compute engine can flag ssa-names that were involved in a range
calculation that have not yet been initialized. When the propagation
for the original name is done, it goes back and examines the "poor
values" and tries to quickly calculate a better range, and if it comes
up with one, immediately tries to go back and update the location/range
gori_compute flagged. This produces better ranges earlier.
However, when we do this in different orders, we can get different
results. We were processing the uses on is_gimple_debug statements just
like normal uses, and this would sometimes cause a difference in how
things were resolved.
This patch adds a flag to enable/disable this attempt to look up new
values, and when range_of_expr is processing the use on a debug
statement, turns it off for the query. This means the query will never
cause a new lookup, and this should resolve all the -fcompare-debug issues.
Bootstrapped on x86_64-pc-linux-gnu, with no new regressions. Pushed.
Please check if such fixes also apply to the GCC 11 branch.
Richard.
I've checked both testcases against gcc11 release, and neither is an
issue there. Much of this was triggered by changes to the export list.
That said, is there potential for it to surface? The potential is
probably there. We'd have to address it differently tho. For the
gcc11 release, since we always run in hybrid mode it doesn't really
matter if ranger looks up ranges for debug statements... EVRP will still
pick up what we use to get for them. we could simply disable looking
for contextual ranges for is_gimple_stmt and simply pick up the best
known global/on-entry value available.. I can either provide a patch
for that now, or deal with it if we ever get a PR. I'm ok either way.
I think it would be good to robustify the code even w/o a PR.
btw, when is the next point release? I added an infrastructure patch to
trunk (https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569884.html)
to enable replacing the on-entry cache to deal with memory consumption
issues like in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100299 . I
specifically put it in early before the other changes so that it could
be directly applied to gcc11 as well, but I need to follow up with one
of the replacements I have queued up to look at if we are interested in
fixing this in gcc 11. I'll bump the priority to try to hit the next
release if thats the case.
The first point release is usuall about two month from the initial release
which means in about a month and a half. It would be nice to fix
those issues and the earlier in the release series the better.
Richard.
Andrew
OK, so this would be the simple way I'd tackle this in gcc11. This
should be quite safe. Just treat debug_stmts as if they are not stmts..
and make a global query. EVRP will still provide a contextual range as
good as it ever did, but it wont trigger ranger lookups on debug uses
any more.
It bootstraps on x86_64-pc-linux-gnu. Is there a process other than
getting the OK to check this into the gcc 11 branch? Does it go into
releases/gcc-11 ?
Andrew
>From ff5ab360b21a83ac84b1fff22d091df2c44dafdf Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacl...@redhat.com>
Date: Tue, 8 Jun 2021 09:43:17 -0400
Subject: [PATCH 4/4] Don't process lookups for debug statements in Ranger.
Although PR 100781 is not an issue in GCC11, its possible that a similar
situation may arise. The identical fix cannot be easily introduced.
With EVRP always running in hybrid mode, there is no need for ranger to
spawn a lookup for a debug statement in this release.
* gimple-range.cc (gimple_ranger::range_of_expr): Treat debug statments
as contextless queries to avoid additional lookups.
---
gcc/gimple-range.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 6158a754dd6..fd7fa5e3dbb 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -945,7 +945,7 @@ gimple_ranger::range_of_expr (irange &r, tree expr, gimple *stmt)
return get_tree_range (r, expr);
// If there is no statement, just get the global value.
- if (!stmt)
+ if (!stmt || is_gimple_debug (stmt))
{
if (!m_cache.get_global_range (r, expr))
r = gimple_range_global (expr);
--
2.25.4
