On 10/21/2021 1:42 AM, Aldy Hernandez wrote:
Massaging the IL should only take two forms IIRC.
First, if we have a simplification we can do. That could be const/copy
propagation, replacing an expression with an SSA_NAME or constant and
the like. It doesn't massage the IL just to massage the IL.
Second, we do temporarily copy propagate the current known values of an
SSA name into use points and then see if that allows us to determine if
a statement is already in the hash tables. But we undo that so that
nobody should see that temporary change in state.
Finally, it does create some expressions & statements on the fly to
enter them into the tables. For example, if it sees a store, it'll
create a load with the source & dest interchanged and enter that into
the expression table. But none of this stuff ever shows up in the IL.
It's just to create entries in the expression tables.
So ITSM the only real concern would be if those temporary const/copy
propagations were still in the IL and we called back into Ranger and it
poked at that data somehow.
Hmmm, this is all very good insight. Thanks.
One thing that would have to be adjusted then is remove the
enable_ranger() call in the patch. This sets a global ranger, and
there are users of get_range_query() that will use it to get on-demand
ranges. One such use that I added was ssa_name_has_boolean_range in
tree-ssa-dom.c. This means that if the IL has been temporarily
changed, this function can and will use the global ranger. The
alternative here would be to just create a new local ranger:
- gimple_ranger *ranger = enable_ranger (fun);
+ gimple_ranger *ranger = new gimple_ranger;
and then obviously deallocate it at the disable_ranger call site.
This will cause any users of get_range_query() in the compiler to just
use global ranges. Hopefully, none of these downstream users of
get_range_query() from DOM need context sensitive results.
ssa_name_has_boolean_range??
I think what you'd need to do is check that there are no calls to the
ranger from cprop_into_stmt (?? this is the place where IL changes??),
until wherever the undoing happens (I couldn't find it). I see a call
to simplify_using_ranges in optimize_stmt that looks like it could be
called with the IL in mid-flight. Maybe this call needs to happen
before the IL is altered?
So if we're referring to those temporary const/copy propagations
"escaping" into Ranger, then I would fully expect that to cause
problems. Essentially they're path sensitive const/copy propagations
and may not be valid on all the paths through the CFG to the statement
where the propagation occurs
Yeah. disabling the global ranger should help, plus making sure you
don't use the ranger in the midst of the path sensitive changes.
I think we should first try to remove those temporary const/copy
propagations. As I noted in a different follow-up, I can't remember if
they were done as part of the original non-copying threader or if they
enabled further optimizations in the copying threader. If its the
former, then they can go away and that would be my preference. I'll try
to poke at that over the weekend.
jeff
