On Wed, Oct 20, 2021 at 10:58 PM Jeff Law <jeffreya...@gmail.com> wrote: > > > > 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.
Are you sure we still do that? I can't find it at least. > > 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 >
