On 5/29/24 03:19, Richard Biener wrote:
On Tue, May 28, 2024 at 8:57 PM Andrew MacLeod <amacl...@redhat.com> wrote:
The original patch causing the PR made ranger's cache re-entrant to
enable SCEV to use the current range_query when called from within ranger..
SCEV uses the currently active range query (via get_range_query()) for
picking up values. fold_using_range is the general purpose stmt folder
many components use, and it takes a range_query to use for folding.
When propagating values in the cache, we need to ensure no new queries
are invoked, and when the cache is propagating and calculating outgoing
edges, it switches to a read only range_query which uses what it knows
about global values to come up with best result using current state.
SCEV is unaware of what the caller is using for a range_query, so when
attempting to fold a PHI node, it is re-invoking the current query
during propagation which is undesired behavior. This patch tells
fold_using_range to not use SCEV if the range_query being used is not
the same as the one SCEV is going to use.
Bootstrapped on x86_64-pc-linux-gnu with no regressions. Pushed.
Can we dump a hint to an active dump-file if this happens? I suppose it's
an unwanted situation, like the pass not setting the active ranger? Sth
like
if (src.query () != get_range_query (cfun)
&& dump_file)
fprintf (dump_file, "Using a range query different from the
installed one\n");
(or better wording).
Sure, I can add that info to the dump. Its unlikely to happen in
places other than from ranger's cache, but you never know.
Btw, could we install src.query () as the global range query around the
relevant recursion or is the place around where we'd need to do this
not so clear-cut?
The problem is actually the other way around. The relevant recursion
location *is* using the global query to try to prevent problems. SCEV
is not honoring the use of an alternative to a non-active range_query
So. An active ranger is the current range_query. Cache propagation of a
ssa-name happens when you ask for a range further down in the CFG than
has been processed yet. It walks the CFG between the last dominator
which has a range calculated, (if any), filling in the on-entry cache
with values for the name based on outgoing edge calculations. it
specifically uses only a global query to fold and evaluate the value of
that name on edges along the way to ensure it doesnt trigger recursion
with the the active ranger. When propagating the cache for an SSA_NAME,
it should finish the propagation and return the final value before it
goes off to do any other activity. Without this patch, when the cache
processes a PHI node via fold_using_ranges(), the generic fold routine
checks with SCEV and SCEV always uses get_range_query instead of a
specific operand source... which invokes the active ranger again :-P
So this patch is mostly to make sure that of we are going to invoke
SCEV, we only invoke it when we are calling it with the same query.
which is 99.9% of the time. If a pass is only using the global range
query, it'll still work fine because get_range_query () returns the
global range query, and thats what SCEV will be using. If a pass uses
enable_ranger() and itself uses get_range_query(), then SCEV will also
work as expected.
Andrew
