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.

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...

...You'll notice that converting to the threader is an exercise in code 
deletion.  Basically, inherit from the hybrid threader while still using 
the copies/avail_exprs business.  This has the added benefit of keeping 
our float threading capabilities intact, while pulling in all the hybrid 
threader goodness.

Over the past year or so, I've been cleaning up evrp clients to use the 
common range_query API.  This makes this conversion easier, as it mostly 
involves replacing evrp_range_analyzer with the ranger and removing the 
state pushing/popping business.

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).

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.

Aldy