On 9/21/21 2:14 AM, Richard Biener wrote:
On Tue, Sep 21, 2021 at 8:09 AM Richard Biener
<richard.guent...@gmail.com> wrote:
On Tue, Sep 21, 2021 at 12:01 AM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
The patch sets the EXECUTABLE property on edges like VRP does, and then
removes that flag when an edge is determined to be un-executable.
This information is then used to return UNDEFINED for any requests on
un-executable edges, and to register equivalencies if all executable
edges of a PHI node are the same SSA_NAME.
This catches up a number of the cases VRP gets that ranger was missing,
and reduces the EVRP discrepancies to almost 0.
On a side note, is there any interest/value in reversing the meaning of
that flag? It seems to me that we could assume edges are EXECUTABLE by
default, then set a NON_EXECUTABLE flag when a pass determines the edge
cannot be executed. This would rpevent a number fo passes from having
to loop through all the edges and set the EXECUTABLE property... It
just seems backwards to me.
The flag is simply not kept up-to-date and it's the passes responsibility to
make use of it (aka install a default state upon entry).
To me not having EDGE_EXECUTABLE set on entry is more natural
for optimistic propagation passes, but yes, if you do on-demand greedy
processing then you need a conservative default. But then how do you
denote a 'VARYING' (executable) state that may not drop back to 'CONSTANT"
(not executable)? For optimistic propagation EDGE_EXECUTABLE set is
simply the varying state and since we never clear it again there's no chance
of oscillation.
Different model, we dont have a lattice whereby we track state and move
form one to another.. we just track currently best known values for
everything and recalculate them when the old values are stale. We move
the edge to unexecutable when those values allow us to rewrite a branch
such that an edge can no longer be taken. everything else is executable.
Any values on an unexecutable edge are then considered UNDEFINED when
combined with other values..
Btw, I fail to see how the patch makes ranger assure a sane initial state of
EDGE_EXECUTABLE (or make its use conditional). Is the code you patched
not also used on-demand?
THe constructor for a ranger makes everything executable to start.
Calls the same routine VRP does.
gimple_ranger::gimple_ranger () : tracer ("")
{
@@ -41,6 +42,7 @@ gimple_ranger::gimple_ranger () : tracer ("")
m_oracle = m_cache.oracle ();
if (dump_file && (param_evrp_mode & EVRP_MODE_TRACE))
tracer.enable_trace ();
+ set_all_edges_as_executable (cfun);
}
and EVRP also goes to this same effort at the start:
evrp_range_analyzer::evrp_range_analyzer (bool update_global_ranges)
: stack (10), m_update_global_ranges (update_global_ranges)
{
edge e;
edge_iterator ei;
basic_block bb;
FOR_EACH_BB_FN (bb, cfun)
{
bb->flags &= ~BB_VISITED;
FOR_EACH_EDGE (e, ei, bb->preds)
e->flags |= EDGE_EXECUTABLE;
}
}
Ranger isn't doing anything different than the ciurrent VRP's do, so I
don't see any distinguishing difference between optimistic and
pessimistic. We can guarantee that we will only clear the EXECUTABLE
flag if the edge is 100% guaranteed to be not executable. Perhaps other
passes/approaches aren't able to provide such guarantees in which case
having a persistent flag wouldnt work very well..
That is, are you sure you're not running into wrong-code issues if you
for example in passes.c:execute_one_pass right before passes
initialize EDGE_EXECUTABLE to a random state on each edge of the function?
Richard.
We dont really have multiple instances running, and I'm not sure that
should be encouraged either. Worst case is that a new instantiation
would "undo" some of the edges that may have been marked as unexecutable
We'd only run into problems if we try to use a ranger from some pass
that is using EXECUTABLE in a different way.. ie,. to not mean
EXECUTABLE. but that seems even more problematic to me, and those uses
ought to be fixed.
Richard.
Anyway, bootstrapped on x86_64-pc-linux-gnu with no regressions. pushed.
Andrew
