From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15299 invoked by alias); 14 Oct 2015 09:28:39 -0000 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 Received: (qmail 15288 invoked by uid 89); 14 Oct 2015 09:28:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f172.google.com Received: from mail-yk0-f172.google.com (HELO mail-yk0-f172.google.com) (209.85.160.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 14 Oct 2015 09:28:36 +0000 Received: by ykaz22 with SMTP id z22so15999471yka.2 for ; Wed, 14 Oct 2015 02:28:34 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.129.125.6 with SMTP id y6mr1343371ywc.5.1444814914638; Wed, 14 Oct 2015 02:28:34 -0700 (PDT) Received: by 10.37.117.136 with HTTP; Wed, 14 Oct 2015 02:28:34 -0700 (PDT) In-Reply-To: References: <20150810082355.GA31149@arm.com> <55C8BFC3.3030603@redhat.com> <55E72D4C.40705@arm.com> <55FC3171.7040509@arm.com> Date: Wed, 14 Oct 2015 09:28:00 -0000 Message-ID: Subject: Re: [PR67891] drop is_gimple_reg test from set_parm_rtl From: Richard Biener To: Alexandre Oliva Cc: =?UTF-8?B?VXJvxaEgQml6amFr?= , Alan Lawrence , Jeff Law , James Greenhalgh , "H.J. Lu" , Segher Boessenkool , GCC Patches , Christophe Lyon , David Edelsohn , Eric Botcazou Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg01330.txt.bz2 On Wed, Oct 14, 2015 at 5:25 AM, Alexandre Oliva wrote: > On Oct 12, 2015, Richard Biener wrote: > >> On Sat, Oct 10, 2015 at 3:16 PM, Alexandre Oliva wrote: >>> On Oct 9, 2015, Richard Biener wrote: >>> >>>> Ok. Note that I think emit_block_move shouldn't mess with the addressable flag. >>> >>> I have successfully tested a patch that stops it from doing so, >>> reverting https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49429#c11 but >>> according to bugs 49429 and 49454, it looks like removing it would mess >>> with escape analysis introduced in r175063 for bug 44194. The thread >>> that introduces the mark_addressable calls suggests some discomfort with >>> this solution, and even a suggestion that the markings should be >>> deferred past the end of expand, but in the end there was agreement to >>> go with it. https://gcc.gnu.org/ml/gcc-patches/2011-06/msg01746.html > >> Aww, indeed. Of course the issue is that we don't track pointers to the >> stack introduced during RTL properly. > >> Thanks for checking. Might want to add a comment before that >> addressable setting now that you've done the archeology. > > I decided to give the following approach a try instead. The following > patch was regstrapped on x86_64-linux-gnu and i686-linux-gnu. > Ok to install? It looks ok to me but lacks a comment in mark_addressable_1 why we do this queueing when currently expanding to RTL. Richard. > Would anyone with access to hpux (pa and ia64 are both affected) give it > a spin? > > > defer mark_addressable calls during expand till the end of expand > > From: Alexandre Oliva > > for gcc/ChangeLog > > * gimple-expr.c: Include hash-set.h and rtl.h. > (mark_addressable_queue): New var. > (mark_addressable): Factor actual marking into... > (mark_addressable_1): ... this. Queue it up during expand. > (mark_addressable_2): New. > (flush_mark_addressable_queue): New. > * gimple-expr.h (flush_mark_addressable_queue): Declare. > * cfgexpand.c: Include gimple-expr.h. > (pass_expand::execute): Flush mark_addressable queue. > --- > gcc/cfgexpand.c | 3 +++ > gcc/gimple-expr.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- > gcc/gimple-expr.h | 1 + > 3 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c > index eaad859..a362e17 100644 > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -51,6 +51,7 @@ along with GCC; see the file COPYING3. If not see > #include "internal-fn.h" > #include "tree-eh.h" > #include "gimple-iterator.h" > +#include "gimple-expr.h" > #include "gimple-walk.h" > #include "cgraph.h" > #include "tree-cfg.h" > @@ -6373,6 +6374,8 @@ pass_expand::execute (function *fun) > /* We're done expanding trees to RTL. */ > currently_expanding_to_rtl = 0; > > + flush_mark_addressable_queue (); > + > FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR_FOR_FN (fun)->next_bb, > EXIT_BLOCK_PTR_FOR_FN (fun), next_bb) > { > diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c > index 2a6ba1a..db249a3 100644 > --- a/gcc/gimple-expr.c > +++ b/gcc/gimple-expr.c > @@ -35,6 +35,8 @@ along with GCC; see the file COPYING3. If not see > #include "gimplify.h" > #include "stor-layout.h" > #include "demangle.h" > +#include "hash-set.h" > +#include "rtl.h" > > /* ----- Type related ----- */ > > @@ -823,6 +825,50 @@ is_gimple_mem_ref_addr (tree t) > || decl_address_invariant_p (TREE_OPERAND (t, 0))))); > } > > +/* Hold trees marked addressable during expand. */ > + > +static hash_set *mark_addressable_queue; > + > +/* Mark X as addressable or queue it up if called during expand. */ > + > +static void > +mark_addressable_1 (tree x) > +{ > + if (!currently_expanding_to_rtl) > + { > + TREE_ADDRESSABLE (x) = 1; > + return; > + } > + > + if (!mark_addressable_queue) > + mark_addressable_queue = new hash_set(); > + mark_addressable_queue->add (x); > +} > + > +/* Adaptor for mark_addressable_1 for use in hash_set traversal. */ > + > +bool > +mark_addressable_2 (tree const &x, void * ATTRIBUTE_UNUSED = NULL) > +{ > + mark_addressable_1 (x); > + return false; > +} > + > +/* Mark all queued trees as addressable, and empty the queue. To be > + called right after clearing CURRENTLY_EXPANDING_TO_RTL. */ > + > +void > +flush_mark_addressable_queue () > +{ > + gcc_assert (!currently_expanding_to_rtl); > + if (mark_addressable_queue) > + { > + mark_addressable_queue->traverse (NULL); > + delete mark_addressable_queue; > + mark_addressable_queue = NULL; > + } > +} > + > /* Mark X addressable. Unlike the langhook we expect X to be in gimple > form and we don't do any syntax checking. */ > > @@ -838,7 +884,7 @@ mark_addressable (tree x) > && TREE_CODE (x) != PARM_DECL > && TREE_CODE (x) != RESULT_DECL) > return; > - TREE_ADDRESSABLE (x) = 1; > + mark_addressable_1 (x); > > /* Also mark the artificial SSA_NAME that points to the partition of X. */ > if (TREE_CODE (x) == VAR_DECL > @@ -849,7 +895,7 @@ mark_addressable (tree x) > { > tree *namep = cfun->gimple_df->decls_to_pointers->get (x); > if (namep) > - TREE_ADDRESSABLE (*namep) = 1; > + mark_addressable_1 (*namep); > } > } > > diff --git a/gcc/gimple-expr.h b/gcc/gimple-expr.h > index 3d1c89f..2917d2752c 100644 > --- a/gcc/gimple-expr.h > +++ b/gcc/gimple-expr.h > @@ -52,6 +52,7 @@ extern bool is_gimple_asm_val (tree); > extern bool is_gimple_min_lval (tree); > extern bool is_gimple_call_addr (tree); > extern bool is_gimple_mem_ref_addr (tree); > +extern void flush_mark_addressable_queue (void); > extern void mark_addressable (tree); > extern bool is_gimple_reg_rhs (tree); > > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer