From patchwork Fri May 17 09:06:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 244544 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id DBEDD2C009E for ; Fri, 17 May 2013 19:07:11 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=VjM8YgCIXVOWpMW5H dtb2f/MrnSZDz9UgkgeoSSRG5b+3gkHdcJ3XLy5JHevgn33JvsqTgbB3ZrdjAPPr dDMY8Evywhp3+Lv5uBuM757N2SCgW/GvQHjJIlhuP8z51bNw07OMQejL/JY5Hz55 kwmfK2r7oNFvIZBA+lWEryUI40= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=4zxOCY1jGIk6r9LCWmvhVCq cfkA=; b=Pf4ZAVbQghzeNeLCI1IoDCvFdFsk0KqYmN4zrwV2IOPdXZbVo6ixhkw bWL1ft4EGtb9TTniF5heLgJ253npMVMNM7nPJDtvv+rNF08wQrBjNVU5AgPrOnh7 1e55f1NlcquAaLkJfiYpPe8fNw7MVE5LiTZOzaZY5tfJtPgIty7w= Received: (qmail 8154 invoked by alias); 17 May 2013 09:07:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 8138 invoked by uid 89); 17 May 2013 09:07:04 -0000 X-Spam-SWARE-Status: No, score=-6.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS, TW_CP autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 17 May 2013 09:07:03 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4H972eu028676 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 17 May 2013 05:07:02 -0400 Received: from redhat.com (ovpn-116-38.ams2.redhat.com [10.36.116.38]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r4H96wH7011430 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 17 May 2013 05:07:01 -0400 Date: Fri, 17 May 2013 11:06:58 +0200 From: Marek Polacek To: Jakub Jelinek Cc: GCC Patches Subject: Re: [PATCH] Don't invalidate string length cache when not needed Message-ID: <20130517090658.GJ14240@redhat.com> References: <20130515165909.GD14240@redhat.com> <20130515172003.GZ1377@tucnak.redhat.com> <20130516133140.GG14240@redhat.com> <20130516133847.GF1377@tucnak.redhat.com> <20130516140744.GH14240@redhat.com> <20130516141827.GG1377@tucnak.redhat.com> <20130516142819.GH1377@tucnak.redhat.com> <20130516164403.GI14240@redhat.com> <20130516203327.GK1377@tucnak.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130516203327.GK1377@tucnak.redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) On Thu, May 16, 2013 at 10:33:27PM +0200, Jakub Jelinek wrote: > On Thu, May 16, 2013 at 06:44:03PM +0200, Marek Polacek wrote: > > --- gcc/tree-ssa-strlen.c.mp 2013-05-15 14:11:20.079707492 +0200 > > +++ gcc/tree-ssa-strlen.c 2013-05-16 17:57:33.963150006 +0200 > > @@ -1693,8 +1693,10 @@ handle_char_store (gimple_stmt_iterator > > } > > else > > { > > - si->writable = true; > > - si->dont_invalidate = true; > > + /* The string might be e.g. in the .rodata section. */ > > + si->writable = false; > > No, this really should be si->writable = true; (and comment not needed). > Ok with that change. Done. > The thing is, while the string might not be known to be writable before, > i.e. we can't optimize away this store, because supposedly it should > trap, if we notice e.g. another write to the same location (writing zero > there again), we can optimize that other write already, because we know > that this store stored there something. > > > +#include "strlenopt.h" > > + > > +__attribute__((noinline, noclone)) size_t > > +fn1 (char *p, const char *r) > > +{ > > + size_t len1 = __builtin_strlen (r); > > + char *q = __builtin_strchr (p, '\0'); > > + *q = '\0'; > > + return len1 - __builtin_strlen (r); // This strlen should be optimized into len1. > > With strlenopt.h include you can avoid using __builtin_ prefixes, all the > builtins needed are prototyped in that header. Ugh, sure, that's only a remnant from my testing. Thanks for the reviews. Will commit this one finally. 2013-05-17 Marek Polacek * tree-ssa-strlen.c (handle_char_store): Don't invalidate cached length when doing non-zero store of storing '\0' to '\0'. * gcc.dg/strlenopt-25.c: New test. * gcc.dg/strlenopt-26.c: Likewise. Marek --- gcc/tree-ssa-strlen.c.mp 2013-05-15 14:11:20.079707492 +0200 +++ gcc/tree-ssa-strlen.c 2013-05-17 10:59:27.461231167 +0200 @@ -1694,7 +1694,8 @@ handle_char_store (gimple_stmt_iterator else { si->writable = true; - si->dont_invalidate = true; + gsi_next (gsi); + return false; } } else @@ -1717,6 +1718,33 @@ handle_char_store (gimple_stmt_iterator si->endptr = ssaname; si->dont_invalidate = true; } + /* If si->length is non-zero constant, we aren't overwriting '\0', + and if we aren't storing '\0', we know that the length of the + string and any other zero terminated string in memory remains + the same. In that case we move to the next gimple statement and + return to signal the caller that it shouldn't invalidate anything. + + This is benefical for cases like: + + char p[20]; + void foo (char *q) + { + strcpy (p, "foobar"); + size_t len = strlen (p); // This can be optimized into 6 + size_t len2 = strlen (q); // This has to be computed + p[0] = 'X'; + size_t len3 = strlen (p); // This can be optimized into 6 + size_t len4 = strlen (q); // This can be optimized into len2 + bar (len, len2, len3, len4); + } + */ + else if (si != NULL && si->length != NULL_TREE + && TREE_CODE (si->length) == INTEGER_CST + && integer_nonzerop (gimple_assign_rhs1 (stmt))) + { + gsi_next (gsi); + return false; + } } else if (idx == 0 && initializer_zerop (gimple_assign_rhs1 (stmt))) { --- gcc/testsuite/gcc.dg/strlenopt-25.c.mp 2013-05-15 17:15:18.702118637 +0200 +++ gcc/testsuite/gcc.dg/strlenopt-25.c 2013-05-15 18:26:27.881030317 +0200 @@ -0,0 +1,18 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fdump-tree-strlen" } */ + +#include "strlenopt.h" + +int +main () +{ + char p[] = "foobar"; + int len, len2; + len = strlen (p); + p[0] = 'O'; + len2 = strlen (p); + return len - len2; +} + +/* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" } } */ +/* { dg-final { cleanup-tree-dump "strlen" } } */ --- gcc/testsuite/gcc.dg/strlenopt-26.c.mp 2013-05-16 17:33:00.302060413 +0200 +++ gcc/testsuite/gcc.dg/strlenopt-26.c 2013-05-17 10:58:17.605015608 +0200 @@ -0,0 +1,25 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fdump-tree-strlen" } */ + +#include "strlenopt.h" + +__attribute__((noinline, noclone)) size_t +fn1 (char *p, const char *r) +{ + size_t len1 = strlen (r); + char *q = strchr (p, '\0'); + *q = '\0'; + return len1 - strlen (r); // This strlen should be optimized into len1. +} + +int +main (void) +{ + char p[] = "foobar"; + const char *volatile q = "xyzzy"; + fn1 (p, q); + return 0; +} + +/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */ +/* { dg-final { cleanup-tree-dump "strlen" } } */