From patchwork Thu Nov 13 21:33:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teresa Johnson X-Patchwork-Id: 410609 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id A5276140079 for ; Fri, 14 Nov 2014 08:33:19 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=GBltNbUmebCdhlPsQk bXl+YN5ZCU8UAcZ1gCzp1l0f51vxXOExDey0qCCL1JBsJdzBAn8btdgki3LZwt/w OcgIgjumeRhz3nQ/jBaGCDx4yA+k813VuLIBfdjKeKebKCfg/XYe3YT8KFiaORid P2NYc2G89Pum5c54NrcEfpLAo= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=aWcnMEaRvXLv1YT9TBylifzk t1A=; b=DfDAlb38rhSDsxik8CKgI5ptz01lrz4h8aidPWOpkP3q9bSRNOGsA2B0 rzhwmaAbsdszRb/VJ5seeszvRRb5B7B5nS2pUCfYSqn3p9afogVhfKLJHFAQOdey VN+FjHCDerD/wwCKLlH6b0iSMwfCBbpy8z+XXNEBckJ0bvEJPYU= Received: (qmail 12130 invoked by alias); 13 Nov 2014 21:33:12 -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 12121 invoked by uid 89); 13 Nov 2014 21:33:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qg0-f53.google.com Received: from mail-qg0-f53.google.com (HELO mail-qg0-f53.google.com) (209.85.192.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 13 Nov 2014 21:33:09 +0000 Received: by mail-qg0-f53.google.com with SMTP id z107so11056447qgd.40 for ; Thu, 13 Nov 2014 13:33:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=zAFCC20q7a/9fpYM4wMvAU1fZF6zbJJGmEoyYsDr3Eg=; b=RiMSSdgWp2oebUWUbLH7S3ph/xfWB1Bu/5WRXgzNmLPfOJs3chDrKeqnhq4QSeGKAN G8Xma7GjyWjwSNBrg1f8XqUhc/9tGBW6gtM6ARzkVab/3YINtRZbWttOWP5GCyKR3IRM kCrLdKUOnM3B/vzg9m/lViILe1TBOepXztmvWowxhX4d3Av1LXtWn8qzSOT96xVTpIEI 0CWTtKq0FhM+NaISdtda1xAH7YNrejzXtRs6IhDxiBSyJ5Ir3BKlW1ROOZQiJotWmiVb LAZo5GVrQdpxmP3b79JWHYqbb1OSA2LPom0bBELWsivjOuUa5ZPzjPOQ7jxupvBZew6N NNcg== X-Gm-Message-State: ALoCoQlmv5gV0LSe/RAkKoJfJ6xwNK7MFXiQJOrIh65ILOyYoV06kafnpWxKEE6b0+80scmkoGMf MIME-Version: 1.0 X-Received: by 10.140.80.136 with SMTP id c8mr6018282qgd.86.1415914387763; Thu, 13 Nov 2014 13:33:07 -0800 (PST) Received: by 10.229.135.68 with HTTP; Thu, 13 Nov 2014 13:33:07 -0800 (PST) In-Reply-To: <20141113205547.GZ5026@tucnak.redhat.com> References: <20141113143617.GR5026@tucnak.redhat.com> <20141113151258.GT5026@tucnak.redhat.com> <20141113205547.GZ5026@tucnak.redhat.com> Date: Thu, 13 Nov 2014 13:33:07 -0800 Message-ID: Subject: Re: [PATCH] Fix PR/63841: empty constructor doesn't zero-initialize From: Teresa Johnson To: Jakub Jelinek Cc: Andrew Pinski , Xinliang David Li , "gcc-patches@gcc.gnu.org" X-IsSubscribed: yes On Thu, Nov 13, 2014 at 12:55 PM, Jakub Jelinek wrote: > On Thu, Nov 13, 2014 at 12:51:05PM -0800, Teresa Johnson wrote: >> On Thu, Nov 13, 2014 at 7:39 AM, Teresa Johnson wrote: >> > On Thu, Nov 13, 2014 at 7:12 AM, Jakub Jelinek wrote: >> >> And for release branches I'd really prefer tree-ssa-strlen.c change. >> > >> > Ok, I started testing the initializer_zerop change on the 4_9 branch, >> > will also test the strlen fix and send that patch for review here when >> > it completes. >> >> Here is the more conservative patch for 4_9. Bootstrapped and tested >> on x86_64-unknown-linux-gnu. Ok for gcc-4_9 branch? > > Ok, thanks. But I have a comment regarding the test below: > >> 2014-11-13 Teresa Johnson >> >> gcc: >> PR tree-optimization/63841 >> * tree-ssa-strlen.c (strlen_optimize_stmt): Ignore clobbers. >> >> 2014-11-13 Teresa Johnson >> >> gcc/testsuite: >> PR tree-optimization/63841 >> * testsuite/g++.dg/tree-ssa/pr63841.C: New test. >> >> Index: tree-ssa-strlen.c >> =================================================================== >> --- tree-ssa-strlen.c (revision 217503) >> +++ tree-ssa-strlen.c (working copy) >> @@ -1856,7 +1856,7 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi) >> break; >> } >> } >> - else if (is_gimple_assign (stmt)) >> + else if (is_gimple_assign (stmt) && !gimple_clobber_p (stmt)) >> { >> tree lhs = gimple_assign_lhs (stmt); >> >> Index: testsuite/g++.dg/tree-ssa/pr63841.C >> =================================================================== >> --- testsuite/g++.dg/tree-ssa/pr63841.C (revision 0) >> +++ testsuite/g++.dg/tree-ssa/pr63841.C (working copy) >> @@ -0,0 +1,38 @@ >> +/* { dg-do run } */ >> +/* { dg-options "-O2" } */ >> + >> +#include >> +#include >> + >> +std::string __attribute__ ((noinline)) comp_test_write() { >> + std::string data; >> + >> + for (int i = 0; i < 2; ++i) { >> + char b = 1 >> (i * 8); >> + data.append(&b, 1); >> + } >> + >> + return data; >> +} >> + >> +std::string __attribute__ ((noinline)) comp_test_write_good() { >> + std::string data; >> + >> + char b; >> + for (int i = 0; i < 2; ++i) { >> + b = 1 >> (i * 8); >> + data.append(&b, 1); >> + } >> + >> + return data; >> +} >> + >> +int main() { >> + std::string good = comp_test_write_good(); >> + printf("expected: %hx\n", *(short*)good.c_str()); >> + >> + std::string bad = comp_test_write(); >> + printf("got: %hx\n", *(short*)bad.c_str()); > > Supposedly the printfs should have been removed and the #include > isn't needed then either. No need to clutter the test output and log files. > On the other side, tests should abort (); or __builtin_abort (); on failure, > not just return non-zero. Ok, sounds good. Here is the new patch. Confirmed that the test case passes with the fix, aborts without it. If this looks good I will also update the trunk version of the test. Thanks, Teresa 2014-11-13 Teresa Johnson gcc: PR tree-optimization/63841 * tree-ssa-strlen.c (strlen_optimize_stmt): Ignore clobbers. 2014-11-13 Teresa Johnson gcc/testsuite: PR tree-optimization/63841 * testsuite/g++.dg/tree-ssa/pr63841.C: New test. > >> + >> + return good != bad; >> +} > > Jakub Index: tree-ssa-strlen.c =================================================================== --- tree-ssa-strlen.c (revision 217503) +++ tree-ssa-strlen.c (working copy) @@ -1856,7 +1856,7 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi) break; } } - else if (is_gimple_assign (stmt)) + else if (is_gimple_assign (stmt) && !gimple_clobber_p (stmt)) { tree lhs = gimple_assign_lhs (stmt); Index: testsuite/g++.dg/tree-ssa/pr63841.C =================================================================== --- testsuite/g++.dg/tree-ssa/pr63841.C (revision 0) +++ testsuite/g++.dg/tree-ssa/pr63841.C (working copy) @@ -0,0 +1,35 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +#include + +std::string __attribute__ ((noinline)) comp_test_write() { + std::string data; + + for (int i = 0; i < 2; ++i) { + char b = 1 >> (i * 8); + data.append(&b, 1); + } + + return data; +} + +std::string __attribute__ ((noinline)) comp_test_write_good() { + std::string data; + + char b; + for (int i = 0; i < 2; ++i) { + b = 1 >> (i * 8); + data.append(&b, 1); + } + + return data; +} + +int main() { + std::string good = comp_test_write_good(); + std::string bad = comp_test_write(); + + if (good != bad) + __builtin_abort (); +}