On Wed, Sep 19, 2018 at 4:19 PM Martin Sebor <mse...@gmail.com> wrote: > > On 09/18/2018 10:23 PM, Jeff Law wrote: > > On 9/18/18 1:46 PM, Martin Sebor wrote: > >> On 09/18/2018 12:58 PM, Jeff Law wrote: > >>> On 9/18/18 11:12 AM, Martin Sebor wrote: > >>> > >>>>> My bad. Sigh. CCP doesn't track copies, just constants, so there's not > >>>>> going to be any data structure you can exploit. And I don't think > >>>>> there's a value number you can use to determine the two objects are the > >>>>> same. > >>>>> > >>>>> Hmm, let's back up a bit, what is does the relevant part of the IL look > >>>>> like before CCP? Is the real problem here that we have unpropagated > >>>>> copies lying around in the IL? Hmm, more likely the IL looksl ike: > >>>>> > >>>>> _8 = &pb_3(D)->a; > >>>>> _9 = _8; > >>>>> _1 = _9; > >>>>> strncpy (MEM_REF (&pb_3(D)->a), ...); > >>>>> MEM[(struct S *)_1].a[n_7] = 0; > >>>> > >>>> Yes, that is what the folder sees while the strncpy call is > >>>> being transformed/folded by ccp. The MEM_REF is folded just > >>>> after the strncpy call and that's when it's transformed into > >>>> > >>>> MEM[(struct S *)_8].a[n_7] = 0; > >>>> > >>>> (The assignments to _1 and _9 don't get removed until after > >>>> the dom walk finishes). > >>>> > >>>>> > >>>>> If we were to propagate the copies out we'd at best have: > >>>>> > >>>>> _8 = &pb_3(D)->a; > >>>>> strncpy (MEM_REF (&pb_3(D)->a), ...); > >>>>> MEM[(struct S *)_8].a[n_7] = 0; > >>>>> > >>>>> > >>>>> Is that in a form you can handle? Or would we also need to forward > >>>>> propagate the address computation into the use of _8? > >>>> > >>>> The above works as long as we look at the def_stmt of _8 in > >>>> the MEM_REF (we currently don't). That's also what the last > >>>> iteration of the loop does. In this case (with _8) it would > >>>> be discovered in the first iteration, so the loop could be > >>>> replaced by a simple if statement. > >>>> > >>>> But I'm not sure I understand the concern with the loop. Is > >>>> it that we are looping at all, i.e., the cost? Or that ccp > >>>> is doing something wrong or suboptimal? (Should have > >>>> propagated the value of _8 earlier?) > >>> I suspect it's more a concern that things like copies are typically > >>> propagated away. So their existence in the IL (and consequently your > >>> need to handle them) raises the question "has something else failed to > >>> do its job earlier". > >>> > >>> During which of the CCP passes is this happening? Can we pull the > >>> warning out of the folder (even if that means having a distinct warning > >>> pass over the IL?) > >> > >> It happens during the third run of the pass. > >> > >> The only way to do what you suggest that I could think of is > >> to defer the strncpy to memcpy transformation until after > >> the warning pass. That was also my earlier suggestion: defer > >> both it and the warning until the tree-ssa-strlen pass (where > >> the warning is implemented to begin with -- the folder calls > >> into it). > > If it's happening that late (CCP3) in general, then ISTM we ought to be > > able to get the warning out of the folder. We just have to pick the > > right spot. > > > > warn_restrict runs before fold_all_builtins, but after dom/vrp so we > > should have the IL in pretty good shape. That seems like about the > > right time. > > > > I wonder if we could generalize warn_restrict to be a more generic > > warning pass over the IL and place it right before fold_builtins. > > The restrict pass doesn't know about string lengths so it can't > handle all the warnings about string built-ins (the strlen pass > now calls into it to issue some). The strlen pass does so it > could handle most if not all of them (the folder also calls > into it to issue some warnings). It would work even better if > it were also integrated with the object size pass. > > We're already working on merging strlen with sprintf. It seems > to me that the strlen pass would benefit not only from that but > also from integrating with object size and warn-restrict. With > that, -Wstringop-overflow could be moved from builtins.c into > it as well (and also benefit not only from accurate string > lengths but also from the more accurate object size info). > > What do you think about that?
I think integrating the various "passes" (objectsize is also as much a facility as a pass) generally makes sense given it might end up improving all of them and reduce code duplication. Richard. > > Martin > > PS I don't think I could do more than merger strlen and sprintf > before stage 1 ends (if even that much) so this would be a longer > term goal.
