On 10/18/2021 2:17 AM, Aldy Hernandez wrote:
On 10/18/21 12:52 AM, Jeff Law wrote:
On 10/8/2021 9:12 AM, Aldy Hernandez via Gcc-patches wrote:
The following patch converts the strlen pass from evrp to ranger,
leaving DOM as the last remaining user.
So is there any reason why we can't convert DOM as well? DOM's use
of EVRP is pretty limited. You've mentioned FP bits before, but my
recollection is those are not part of the EVRP analysis DOM uses.
Hell, give me a little guidance and I'll do the work...
Not only will I take you up on that offer, but I can provide 90% of
the work. Here be dragons, though (well, for me, maybe not for you ;-)).
DOM is actually an evrp pass at -O1 in disguise. The reason it really
is a covert evrp pass is because:
a) It calls extract_range_from_stmt on each statement.
b) It folds conditionals with simplify_using_ranges.
c) But most importantly, it exports discovered ranges when it's done
(evrp_range_analyzer(true)).
If you look at the evrp pass, you'll notice that that's basically what
it does, albeit with the substitute and fold engine, which also calls
gimple fold plus other goodies.
But I could argue that we've made DOM into an evrp pass without
noticing. The last item (c) is particularly invasive because these
exported ranges show up in other passes unexpectedly. For instance, I
saw an RTL pass at -O1 miss an optimization because it was dependent
on some global range being set. IMO, DOM should not export global
ranges it discovered during its walk (do one thing and do it well),
but I leave it to you experts to pontificate.
All true. But I don't think we've got many, if any, hard dependencies
on those behaviors.
The attached patch is rather trivial. It's mostly deleting state. It
seems DOM spends a lot of time massaging the IL so that it can fold
conditionals or thread paths. None of this is needed, because the
ranger can do all of this. Well, except floats, but...
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.
That's the good news. The bad news is that DOM changes the IL as it
goes and the patch doesn't bootstrap. Andrew insists that we should
work even with DOM's changing IL, but last time we played this dance
with the substitute_and_fold engine, there were some tweaks needed to
the ranger. Could be this, but I haven't investigated. It could also
be that the failures I was seeing were just DOM things that were no
longer needed (shuffling the IL to simplify things for evrp).
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
This just needs a little shepherding from a DOM expert ;-). If you
get it to bootstrap, I could take care of the tests, performance, and
making sure we're getting the same number of threads etc.
No additional cleanups have been done. For example, the strlen pass
still has uses of VR_ANTI_RANGE, and the sprintf still passes around
pairs of integers instead of using a proper range. Fixing this
could further improve these passes.
As a further enhancement, if the relevant maintainers deem useful,
the domwalk could be removed from strlen. That is, unless the pass
needs it for something else.
The dom walk was strictly for the benefit of EVRP when it was added.
So I think it can get zapped once the pass is converted.
Jakub mentioned a while ago, that the strlen pass itself needs DOM, so
perhaps this needs to stay.
Yes, strlen wants DOM. But the printf bits don't really need it. Well,
maybe they do now that the printf warnings are more tightly integrated
with the strlen bits.
jeff
