From patchwork Fri Jan 16 15:38:29 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Enkovich X-Patchwork-Id: 429895 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 A51781401EB for ; Sat, 17 Jan 2015 02:38:57 +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:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=S1RB5q5CBuvSOKcss JrU8GKmvIOnGzGlXrCHMNUTx6IRkcxj8CJcdymSZ/isGcSkP0F/U2I9KcIuwxd9S ZQ3oDqgmWnYHo6hFyOuT7Eo3G4cPICr0EQO+5jdM4/ItEbWjDVdRYnMkP/VLI18z xa9gaL1Z8CjqoLCl1P+7HMjeDM= 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=mgI5gDbVq17he9r7VHjQl4D xpaU=; b=K4POQ1BjArTmDhpnkkdlAO4n280wpBIIwlzK4kJ+RWIJ7IB0TZi7Han 40e4MCV/0ZKxQyESYfN9Ej6zmklASuUNM//1QtJp3XG5gVrmahKY1WQaelpN0Rz9 Bi0sCwk1KI1hvTLfyr5t4DZAuCieOCAzZZLUGZabIRMFLRqW0Xgo= Received: (qmail 30975 invoked by alias); 16 Jan 2015 15:38:50 -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 30965 invoked by uid 89); 16 Jan 2015 15:38:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f177.google.com Received: from mail-wi0-f177.google.com (HELO mail-wi0-f177.google.com) (209.85.212.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 16 Jan 2015 15:38:47 +0000 Received: by mail-wi0-f177.google.com with SMTP id l15so4629118wiw.4 for ; Fri, 16 Jan 2015 07:38:44 -0800 (PST) X-Received: by 10.194.77.73 with SMTP id q9mr29869686wjw.24.1421422723931; Fri, 16 Jan 2015 07:38:43 -0800 (PST) Received: from msticlxl57.ims.intel.com (fmdmzpr02-ext.fm.intel.com. [192.55.55.37]) by mx.google.com with ESMTPSA id 18sm6492627wjr.46.2015.01.16.07.38.41 (version=TLSv1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 16 Jan 2015 07:38:43 -0800 (PST) Date: Fri, 16 Jan 2015 18:38:29 +0300 From: Ilya Enkovich To: Richard Biener Cc: GCC Patches Subject: Re: [PATCH] Fix for PR64353 Message-ID: <20150116153829.GD55666@msticlxl57.ims.intel.com> References: <20150114131904.GA56209@msticlxl57.ims.intel.com> <20150114161258.GB56209@msticlxl57.ims.intel.com> <269EA1AC-A67F-47CB-9CA3-14FA4A68FF98@gmail.com> <20150116105112.GA55666@msticlxl57.ims.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On 16 Jan 12:38, Richard Biener wrote: > On Fri, Jan 16, 2015 at 11:51 AM, Ilya Enkovich wrote: > > On 14 Jan 19:40, Richard Biener wrote: > >> On January 14, 2015 5:23:21 PM CET, Ilya Enkovich wrote: > >> >On 14 Jan 15:35, Richard Biener wrote: > >> >> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich > >> > wrote: > >> >> > Hi, > >> >> > > >> >> > SRA gimple passes may add loads to functions with no SSA update. > >> >Later it causes ICE when function with not updated SSA is processed by > >> >gimple passes. This patch fixes it by calling update_ssa. > >> >> > > >> >> > Bootstrapped and checked on x86_64-unknown-linux-gnu. OK for > >> >trunk? > >> >> > >> >> No. I have removed this quadratic update-ssa call previously. It > >> >should > >> >> simply keep SSA for up-to-date manually (see how it does > >> >gimple_set_vuse > >> >> in some cases, probably not all required ones?). > >> >> > >> > > >> >Would it be OK to call update_ssa only in case we don't have a proper > >> >VUSE for call? > >> > >> No, and most definitely not here. > >> > >> > >> Are we allowed to just emit error due to incorrect > >> >attribute? > >> > >> No, I don't think so either. But we may drop it. > >> > >> Richard. > >> > > > > Here is a version with SRA disabled for functions with __attribute__((const)) as you suggested in tracker. Is it OK? > > > > Bootstrapped and checked on x86_64-unknown-linux-gnu. > > > > Thanks, > > Ilya > > -- > > gcc/ > > > > 2015-01-16 Ilya Enkovich > > > > PR middle-end/64353 > > * tree-sra.c (ipa_sra_preliminary_function_checks): Reject > > functions with const attribute. > > > > gcc/testsuite/ > > > > 2015-01-16 Ilya Enkovich > > > > PR middle-end/64353 > > * g++.dg/pr64353.C: New. > > > > > > diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C > > new file mode 100644 > > index 0000000..7859918 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pr64353.C > > @@ -0,0 +1,15 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2" } */ > > + > > +class C > > +{ > > + int y, x; > > + void i (); > > + bool __attribute__((const)) xx () { return x; } > > +}; > > + > > +void C::i () > > +{ > > + if (xx ()) > > + x = 1; > > +} > > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > > index f560fe0..f24ca9f 100644 > > --- a/gcc/tree-sra.c > > +++ b/gcc/tree-sra.c > > @@ -5015,6 +5015,13 @@ ipa_sra_preliminary_function_checks (struct cgraph_node *node) > > return false; > > } > > > > + if (lookup_attribute ("const", DECL_ATTRIBUTES (node->decl))) > > TREE_READONLY (node->decl) > > but I'm now not sure that we can really trust this given that only fixup_cfg > will eventually fixup stmts in callers after ipa-pure-const ran. The above > also doesn't handle "no vops" functions. > > I think a better place to check this would be inside > some_callers_have_mismatched_arguments_p where you'd check > > || !gimple_vuse (cs->call_stmt) > > of course the reasoning printed is wrong then. > > We can fix this by running update-ssa in fixup_cfg as well, and I guess > I like that more. > > Index: tree-cfg.c > =================================================================== > --- tree-cfg.c (revision 219714) > +++ tree-cfg.c (working copy) > @@ -8754,7 +8804,7 @@ const pass_data pass_data_fixup_cfg = > PROP_cfg, /* properties_required */ > 0, /* properties_provided */ > 0, /* properties_destroyed */ > - 0, /* todo_flags_start */ > + TODO_update_ssa_only_virtuals, /* todo_flags_start */ > 0, /* todo_flags_finish */ > }; > > this variant is pre-approved if it passes bootstrap and regtest. > > Thanks, > Richard. > Bootstrap and regtest is OK on x86_64-unknown-linux-gnu. Here is a committed patch. Thanks, Ilya --- gcc/ 2015-01-16 Ilya Enkovich PR middle-end/64353 * tree-cfg.c (pass_data_fixup_cfg): Update SSA for virtuals on start. gcc/testsuite/ 2015-01-16 Ilya Enkovich PR middle-end/64353 * g++.dg/pr64353.C: New. diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C new file mode 100644 index 0000000..7859918 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr64353.C @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +class C +{ + int y, x; + void i (); + bool __attribute__((const)) xx () { return x; } +}; + +void C::i () +{ + if (xx ()) + x = 1; +} diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index a9a2c2f..b56a453 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -8741,7 +8741,7 @@ const pass_data pass_data_fixup_cfg = PROP_cfg, /* properties_required */ 0, /* properties_provided */ 0, /* properties_destroyed */ - 0, /* todo_flags_start */ + TODO_update_ssa_only_virtuals, /* todo_flags_start */ 0, /* todo_flags_finish */ };