Re: PR79715: Special case strcpy/strncpy for dse

[Martin Sebor <msebor at gmail dot com>]

On 04/24/2017 03:05 AM, Prathamesh Kulkarni wrote:
> On 1 March 2017 at 13:24, Richard Biener <rguenther@suse.de> wrote:
> > On Tue, 28 Feb 2017, Jeff Law wrote:
> >
> > > On 02/28/2017 05:59 AM, Prathamesh Kulkarni wrote:
> > > > On 28 February 2017 at 15:40, Jakub Jelinek <jakub@redhat.com> wrote:
> > > > > On Tue, Feb 28, 2017 at 03:33:11PM +0530, Prathamesh Kulkarni wrote:
> > > > > > Hi,
> > > > > > The attached patch adds special-casing for strcpy/strncpy to dse pass.
> > > > > > Bootstrapped+tested on x86_64-unknown-linux-gnu.
> > > > > > OK for GCC 8 ?
> > > > >
> > > > > What is special on strcpy/strncpy?  Unlike memcpy/memmove/memset, you
> > > > > don't
> > > > > know the length they store (at least not in general), you don't know the
> > > > > value, all you know is that they are for the first argument plain store
> > > > > without remembering the pointer or anything based on it anywhere except in
> > > > > the return value.
> > > > > I believe stpcpy, stpncpy, strcat, strncat at least have the same
> > > > > behavior.
> > > > > On the other side, without knowing the length of the store, you can't
> > > > > treat
> > > > > it as killing something (ok, some calls like strcpy or stpcpy store at
> > > > > least
> > > > > the first byte).
> > > > Well, I assumed a store to dest by strcpy (and friends), which gets
> > > > immediately freed would count
> > > > as a dead store since free would kill the whole memory block pointed
> > > > to by dest ?
> > > Yes.  But does it happen often in practice?  I wouldn't mind exploring this
> > > for gcc-8, but I'd like to see real-world code where this happens.
> >
> > Actually I don't mind for "real-world code" - the important part is
> > that I believe it's reasonable to assume it can happen from some C++
> > abstraction and optimization.
> Hi,
> I have updated the patch to include stp[n]cpy and str[n]cat.
> In initialize_ao_ref_for_dse for strncat, I suppose for strncat we
> need to treat size as NULL_TREE
> instead of setting it 2nd arg since we cannot know size (in general)
> after strncat ?
> Patch passes bootstrap+test on x86_64-unknown-linux-gnu.
> Cross-tested on arm*-*-*, aarch64*-*-*.

I think I made a mistake when I opened PR79715: I used the wrong
-fdump-tree- option and was looking at the wrong dump.  It turns
out that the test case there is optimized as I expected, except
by a later pass, and has been since r233267.  I resolved the bug
as fixed before I realized that this patch (that I've been meaning
to review) is for that bug, except that it enables the optimization
to take place earlier.  That sounds like a fine idea to me but now
that I closed the bug I wonder if it should be done under a different
bug.  This is just a suggestion and if you don't want to take the
time to raise a new bug for I don't mind if you reopen bug 79715.
Sorry if I made things more complicated than they need to be.

Martin