From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23162 invoked by alias); 12 Feb 2013 14:05:21 -0000 Received: (qmail 23061 invoked by uid 22791); 12 Feb 2013 14:05:19 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 Feb 2013 14:05:10 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1CE59Rb023551 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 12 Feb 2013 09:05:09 -0500 Received: from zalov.redhat.com (vpn1-5-178.ams2.redhat.com [10.36.5.178]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1CE55Xn007050 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 12 Feb 2013 09:05:06 -0500 Received: from zalov.cz (localhost [127.0.0.1]) by zalov.redhat.com (8.14.5/8.14.5) with ESMTP id r1CE54AY030001; Tue, 12 Feb 2013 15:05:04 +0100 Received: (from jakub@localhost) by zalov.cz (8.14.5/8.14.5/Submit) id r1CE54Qt030000; Tue, 12 Feb 2013 15:05:04 +0100 Date: Tue, 12 Feb 2013 14:05:00 -0000 From: Jakub Jelinek To: Aldy Hernandez , Richard Sandiford Cc: gcc-patches Subject: Re: PR target/52555: attribute optimize is overriding command line options Message-ID: <20130212140503.GD4385@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <51198989.7090803@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51198989.7090803@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-02/txt/msg00531.txt.bz2 On Mon, Feb 11, 2013 at 06:15:05PM -0600, Aldy Hernandez wrote: > How does this look? Looks good to me. > Jakub, what's this you mention in the PR about caching > __optimize__((3))? You also mention I shouldn't compare against > this_target_optabs, but default_target_optabs. But what if > this_target_optabs has changed? (See patch). The reason for that is that this_target_optabs could at that point be simply whatever optabs used the last parsed function. this_target_optabs changes only either because of optimize attribute (not sure if MIPS as the only switchable target? supports that), or because of mips_set_mips16_mode. I think invoke_set_current_function_hook invokes the target hook after the code you've changed, so I'd say it should work fine even on MIPS. CCing Richard for that anyway. > > Tested on x86-64 Linux. It would be nice if someone with access to > a SWITCHABLE_TARGET platform (mips) could also test this. A few nits below. I'm not sure about the behavior of multiple optimize attributes either, let's try it and see how it is handled right now (ignoring optabs). > @@ -6184,6 +6184,41 @@ init_optabs (void) > targetm.init_libfuncs (); > } > > +/* Recompute the optabs. If they have changed, save the new set of > + optabs in the optimization node OPTNODE. */ > + > +void > +save_optabs_if_changed (tree optnode) > +{ > + struct target_optabs *save_target_optabs = this_target_optabs; > + struct target_optabs *tmp_target_optabs = XNEW (struct target_optabs); > + > + /* Generate a new set of optabs into tmp_target_optabs. */ > + memset (tmp_target_optabs, 0, sizeof (struct target_optabs)); I think you should just use XCNEW and drop the memset. > + this_target_optabs = tmp_target_optabs; > + init_all_optabs (); > + this_target_optabs = save_target_optabs; > + > + /* If the optabs changed, record it in the node. */ > + if (memcmp (tmp_target_optabs, this_target_optabs, > + sizeof (struct target_optabs))) > + { > + /* ?? An existing entry in TREE_OPTIMIZATION_OPTABS indicates > + multiple ((optimize)) attributes for the same function. Is > + this even valid? For now, just clobber the existing entry > + with the new optabs. */ > + if (TREE_OPTIMIZATION_OPTABS (optnode)) > + free (TREE_OPTIMIZATION_OPTABS (optnode)); > + > + TREE_OPTIMIZATION_OPTABS (optnode) = tmp_target_optabs; > + } > + else > + { > + TREE_OPTIMIZATION_OPTABS (optnode) = NULL; > + free (tmp_target_optabs); Shouldn't this (and above) be XDELETE to match the allocation style? Jakub