On Thu, Oct 21, 2021 at 3:30 PM Aldy Hernandez <al...@redhat.com> wrote:
>
>
>
> On 10/21/21 3:14 PM, Richard Biener wrote:
> > On Thu, Oct 21, 2021 at 2:56 PM Aldy Hernandez <al...@redhat.com> wrote:
> >>
> >> On Thu, Oct 21, 2021 at 12:20 PM Richard Biener
> >> <richard.guent...@gmail.com> wrote:
> >>>
> >>> 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.
> >>
> >> I couldn't either, but perhaps what Jeff is referring to is the ad-hoc
> >> copy propagation that happens in the DOM's use of the threader:
> >>
> >> /* Make a copy of the uses & vuses into USES_COPY, then cprop into
> >> the operands. */
> >> FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES)
> >> {
> >> tree tmp = NULL;
> >> tree use = USE_FROM_PTR (use_p);
> >>
> >> copy[i++] = use;
> >> if (TREE_CODE (use) == SSA_NAME)
> >> tmp = SSA_NAME_VALUE (use);
> >> if (tmp)
> >> SET_USE (use_p, tmp);
> >> }
> >>
> >> cached_lhs = simplifier->simplify (stmt, stmt, bb, this);
> >>
> >> /* Restore the statement's original uses/defs. */
> >> i = 0;
> >> FOR_EACH_SSA_USE_OPERAND (use_p, stmt, iter, SSA_OP_ALL_USES)
> >> SET_USE (use_p, copy[i++]);
> >
> > Ah, likely. These days we'd likely use a gimple_match_op but then
> > this seems heavily abstracted, no idea where simplifier->simplify
> > might lead to ;)
> > I'm also not sure why the threader would do the valueization here and
> > not the simplify() function (and lookup_avail_expr misses an 'exploded'
> > operand
> > lookup as well). Lot's of legacy code ;)
>
> Yes, there's a lot of legacy code I've left mostly alone.
>
> There are two copies of simplify() now, the DOM version
> (dom_jt_simplifier::simplify) and the VRP version
> (hybrid_jt_simplifier::simplify). Each do slightly different things, as
> has always been the case. It's just that the separation is now explicit.
>
> No idea what's up with the valueization either. The VRP version doesn't
> need any of this valuezation or on the side structures. You just ask
> the range of a stmt on a path and it gives you an answer.
Yeah, but it doesn't "simplify", it uses ranger to do constant folding
... (ick).
For random expressions I'd have used gimple_simplify (in fact it somehow
tries to do value-numbering or sth like that to derive new equivalences).
> >
> > But I think the above is not going to be an issue unless ranger runs in
> > circles around backedges, arriving at this very same stmt again? A way
> > out might be to copy the stmt to the stack, adjust operands and use that
> > for the simplify entry.
>
> If you look at the patch I sent Jeff, you'll see that I've removed most
> of what's in that function. There's no need to propagate anything. The
> ranger can simplify the final conditional without having to set up anything.
So maybe we can remove all that code (jt_state::register_equivs_stmt).
Richard.
> Aldy
>
