From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26497 invoked by alias); 15 Aug 2012 12:07:54 -0000 Received: (qmail 26463 invoked by uid 22791); 15 Aug 2012 12:07:52 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 15 Aug 2012 12:07:37 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 7D6B4A39D0; Wed, 15 Aug 2012 14:07:36 +0200 (CEST) Date: Wed, 15 Aug 2012 12:07:00 -0000 From: Richard Guenther To: Ulrich Weigand Cc: gcc-patches@gcc.gnu.org Subject: Re: [rfc] Fix SPU build (Re: [PATCH] Remove basic_block->loop_depth) In-Reply-To: <201208151204.q7FC4N7p000624@d06av02.portsmouth.uk.ibm.com> Message-ID: References: <201208151204.q7FC4N7p000624@d06av02.portsmouth.uk.ibm.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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: 2012-08/txt/msg00964.txt.bz2 On Wed, 15 Aug 2012, Ulrich Weigand wrote: > Richard Guenther wrote: > > On Tue, 14 Aug 2012, Ulrich Weigand wrote: > > > Looks like this broke SPU build, since spu_machine_dependent_reorg > > > accesses ->loop_depth. According to comments in the code, this > > > was done because of concerns that loop_father may no longer be set up > > > this late in compilation, so I'm wondering whether just replacing > > > this by loop_depth (bb->loop_father) would work here ... > > > If SPU md reorg would like to look at loop structures it should > > compute them. Simply call flow_loops_find, which hopefully works > > in CFG RTL mode (which I think is the mode available from md reorg?). > > It seems flow_loops_find by itself is not quite enough, but everything > necessary to use the loop structures seems to be encapsulated in > loop_optimizer_init / loop_optimizer_finalize, which are already called > by a number of optimization passes. When I do the same in the SPU > md-reorg pass, loop_father seems to be set up OK. > > Does this look reasonable? Yes. You can use bb_loop_depth instead of loop_depth (bb->loop_father). Richard. > Thanks, > Ulrich > > > ChangeLog: > > * config/spu/spu.c: Include "cfgloop.h". > (spu_machine_dependent_reorg): Call loop_optimizer_init and > loop_optimizer_finalize. Use loop_father instead of loop_depth. > * config/spu/t-spu-elf (spu.o): Update dependencies. > > Index: gcc/config/spu/spu.c > =================================================================== > *** gcc/config/spu/spu.c (revision 190390) > --- gcc/config/spu/spu.c (working copy) > *************** > *** 53,58 **** > --- 53,59 ---- > #include "timevar.h" > #include "df.h" > #include "dumpfile.h" > + #include "cfgloop.h" > > /* Builtin types, data and prototypes. */ > > *************** spu_machine_dependent_reorg (void) > *** 2458,2463 **** > --- 2459,2468 ---- > in_spu_reorg = 1; > compute_bb_for_insn (); > > + /* (Re-)discover loops so that bb->loop_father can be used > + in the analysis below. */ > + loop_optimizer_init (AVOID_CFG_MODIFICATIONS); > + > compact_blocks (); > > spu_bb_info = > *************** spu_machine_dependent_reorg (void) > *** 2562,2575 **** > fallthru block. This catches the cases when it is a simple > loop or when there is an initial branch into the loop. */ > if (prev && (loop_exit || simple_loop) > ! && prev->loop_depth <= bb->loop_depth) > prop = prev; > > /* If there is only one adjacent predecessor. Don't propagate > ! outside this loop. This loop_depth test isn't perfect, but > ! I'm not sure the loop_father member is valid at this point. */ > else if (prev && single_pred_p (bb) > ! && prev->loop_depth == bb->loop_depth) > prop = prev; > > /* If this is the JOIN block of a simple IF-THEN then > --- 2567,2580 ---- > fallthru block. This catches the cases when it is a simple > loop or when there is an initial branch into the loop. */ > if (prev && (loop_exit || simple_loop) > ! && (loop_depth (prev->loop_father) > ! <= loop_depth (bb->loop_father))) > prop = prev; > > /* If there is only one adjacent predecessor. Don't propagate > ! outside this loop. */ > else if (prev && single_pred_p (bb) > ! && prev->loop_father == bb->loop_father) > prop = prev; > > /* If this is the JOIN block of a simple IF-THEN then > *************** spu_machine_dependent_reorg (void) > *** 2578,2584 **** > && EDGE_COUNT (bb->preds) == 2 > && EDGE_COUNT (prev->preds) == 1 > && EDGE_PRED (prev, 0)->src == prev2 > ! && prev2->loop_depth == bb->loop_depth > && GET_CODE (branch_target) != REG) > prop = prev; > > --- 2583,2589 ---- > && EDGE_COUNT (bb->preds) == 2 > && EDGE_COUNT (prev->preds) == 1 > && EDGE_PRED (prev, 0)->src == prev2 > ! && prev2->loop_father == bb->loop_father > && GET_CODE (branch_target) != REG) > prop = prev; > > *************** spu_machine_dependent_reorg (void) > *** 2600,2606 **** > if (dump_file) > fprintf (dump_file, "propagate from %i to %i (loop depth %i) " > "for %i (loop_exit %i simple_loop %i dist %i)\n", > ! bb->index, prop->index, bb->loop_depth, > INSN_UID (branch), loop_exit, simple_loop, > branch_addr - INSN_ADDRESSES (INSN_UID (bbend))); > > --- 2605,2611 ---- > if (dump_file) > fprintf (dump_file, "propagate from %i to %i (loop depth %i) " > "for %i (loop_exit %i simple_loop %i dist %i)\n", > ! bb->index, prop->index, loop_depth (bb->loop_father), > INSN_UID (branch), loop_exit, simple_loop, > branch_addr - INSN_ADDRESSES (INSN_UID (bbend))); > > *************** spu_machine_dependent_reorg (void) > *** 2657,2662 **** > --- 2662,2669 ---- > > spu_var_tracking (); > > + loop_optimizer_finalize (); > + > free_bb_for_insn (); > > in_spu_reorg = 0; > Index: gcc/config/spu/t-spu-elf > =================================================================== > *** gcc/config/spu/t-spu-elf (revision 190390) > --- gcc/config/spu/t-spu-elf (working copy) > *************** spu.o: $(CONFIG_H) $(SYSTEM_H) coretypes > *** 23,29 **** > real.h insn-config.h conditions.h insn-attr.h flags.h $(RECOG_H) \ > $(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \ > output.h $(BASIC_BLOCK_H) $(GGC_H) $(HASHTAB_H) \ > ! $(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h \ > $(srcdir)/config/spu/spu-protos.h \ > $(srcdir)/config/spu/spu-builtins.def > > --- 23,29 ---- > real.h insn-config.h conditions.h insn-attr.h flags.h $(RECOG_H) \ > $(OBSTACK_H) $(TREE_H) $(EXPR_H) $(OPTABS_H) except.h function.h \ > output.h $(BASIC_BLOCK_H) $(GGC_H) $(HASHTAB_H) \ > ! $(TM_P_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h reload.h $(CFGLOOP_H) \ > $(srcdir)/config/spu/spu-protos.h \ > $(srcdir)/config/spu/spu-builtins.def