[PATCH] Convert strlen pass from evrp to ranger.

[Aldy Hernandez]

On 10/21/21 3:43 PM, Jeff Law wrote:
> 
> 
> On 10/21/2021 6:56 AM, Aldy Hernandez wrote:
>> On Thu, Oct 21, 2021 at 12:20 PM Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Oct 20, 2021 at 10:58 PM Jeff Law <jeffreyalaw@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++]);
> Exactly what I was referring to.  It may be worth an experiment -- I 
> can't recall when this code went in.  It might be a remnant from the 
> original threader that pre-dates block copying.  In that world we had to 
> look up expressions in the table as part of the step to verify that we 
> could safely ignore statements at the start of a block.
> 
> Or it could be something that was added to improve threading when the 
> condition we're trying to thread through is partially redundant on the 
> thread path.  This would allow us to discover that partial redundancy 
> and exploit it for threading.
> 
> In either case this code may have outlived its usefulness.  I wonder 
> what would happen if we just took it out.

Look at the patch I sent you.  I rip most of it out, as ranger doesn't 
need it.  Perhaps DOM+evrp+threader doesn't either??

Aldy