From patchwork Sun May 29 18:03:37 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 97846 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]) by ozlabs.org (Postfix) with SMTP id 22CD0B6F82 for ; Mon, 30 May 2011 04:04:10 +1000 (EST) Received: (qmail 23904 invoked by alias); 29 May 2011 18:04:08 -0000 Received: (qmail 23892 invoked by uid 22791); 29 May 2011 18:04:07 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RFC_ABUSE_POST, T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 29 May 2011 18:03:45 +0000 Received: by wwf26 with SMTP id 26so3003922wwf.8 for ; Sun, 29 May 2011 11:03:43 -0700 (PDT) Received: by 10.216.221.29 with SMTP id q29mr1709074wep.6.1306692223793; Sun, 29 May 2011 11:03:43 -0700 (PDT) Received: from localhost (rsandifo.gotadsl.co.uk [82.133.89.107]) by mx.google.com with ESMTPS id gb6sm2536320wbb.51.2011.05.29.11.03.42 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 29 May 2011 11:03:42 -0700 (PDT) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, rdsandiford@googlemail.com Subject: [committed] PR target/43995: infinite recursion in mips_reorg Date: Sun, 29 May 2011 19:03:37 +0100 Message-ID: <87vcwtjsee.fsf@firetop.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 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 mips_pic_call_symbol_from_set has code to follow register sets. In PR target/43995, a dependency loop makes this code recurse infinitely. I considered rewriting the code to use a worklist, possibly with results cached between call sites. However, we should only really need to follow one register copy, so that seemed like overkill. It would also have been too invasive for the release branches. I instead went for the brute-force option of limiting recursion to the expected single set. Tested on mips-linux-gnu. Applied to trunk, 4.6 and 4.5. Richard gcc/ PR target/43995 * config/mips/mips.c (mips_pic_call_symbol_from_set): Add a recurse_p argument. Only follow register copies if it is set, and prevent mips_find_pic_call_symbol from recursing. (mips_find_pic_call_symbol): Add a recurse_p argument. Pass it to mips_pic_call_symbol_from_set. (mips_annotate_pic_calls): Update accordingly. Index: gcc/config/mips/mips.c =================================================================== --- gcc/config/mips/mips.c 2011-05-28 17:08:48.000000000 +0100 +++ gcc/config/mips/mips.c 2011-05-28 18:10:55.000000000 +0100 @@ -1060,7 +1060,7 @@ static const struct mips_rtx_cost_data m } }; -static rtx mips_find_pic_call_symbol (rtx, rtx); +static rtx mips_find_pic_call_symbol (rtx, rtx, bool); static int mips_register_move_cost (enum machine_mode, reg_class_t, reg_class_t); static unsigned int mips_function_arg_boundary (enum machine_mode, const_tree); @@ -14026,12 +14026,16 @@ mips_call_expr_from_insn (rtx insn, rtx } /* REG is set in DEF. See if the definition is one of the ways we load a - register with a symbol address for a mips_use_pic_fn_addr_reg_p call. If - it is return the symbol reference of the function, otherwise return - NULL_RTX. */ + register with a symbol address for a mips_use_pic_fn_addr_reg_p call. + If it is, return the symbol reference of the function, otherwise return + NULL_RTX. + + If RECURSE_P is true, use mips_find_pic_call_symbol to interpret + the values of source registers, otherwise treat such registers as + having an unknown value. */ static rtx -mips_pic_call_symbol_from_set (df_ref def, rtx reg) +mips_pic_call_symbol_from_set (df_ref def, rtx reg, bool recurse_p) { rtx def_insn, set; @@ -14058,21 +14062,39 @@ mips_pic_call_symbol_from_set (df_ref de return symbol; } - /* Follow simple register copies. */ - if (REG_P (src)) - return mips_find_pic_call_symbol (def_insn, src); + /* Follow at most one simple register copy. Such copies are + interesting in cases like: + + for (...) + { + locally_binding_fn (...); + } + + and: + + locally_binding_fn (...); + ... + locally_binding_fn (...); + + where the load of locally_binding_fn can legitimately be + hoisted or shared. However, we do not expect to see complex + chains of copies, so a full worklist solution to the problem + would probably be overkill. */ + if (recurse_p && REG_P (src)) + return mips_find_pic_call_symbol (def_insn, src, false); } return NULL_RTX; } -/* Find the definition of the use of REG in INSN. See if the definition is - one of the ways we load a register with a symbol address for a - mips_use_pic_fn_addr_reg_p call. If it is return the symbol reference of - the function, otherwise return NULL_RTX. */ +/* Find the definition of the use of REG in INSN. See if the definition + is one of the ways we load a register with a symbol address for a + mips_use_pic_fn_addr_reg_p call. If it is return the symbol reference + of the function, otherwise return NULL_RTX. RECURSE_P is as for + mips_pic_call_symbol_from_set. */ static rtx -mips_find_pic_call_symbol (rtx insn, rtx reg) +mips_find_pic_call_symbol (rtx insn, rtx reg, bool recurse_p) { df_ref use; struct df_link *defs; @@ -14084,7 +14106,7 @@ mips_find_pic_call_symbol (rtx insn, rtx defs = DF_REF_CHAIN (use); if (!defs) return NULL_RTX; - symbol = mips_pic_call_symbol_from_set (defs->ref, reg); + symbol = mips_pic_call_symbol_from_set (defs->ref, reg, recurse_p); if (!symbol) return NULL_RTX; @@ -14093,7 +14115,7 @@ mips_find_pic_call_symbol (rtx insn, rtx { rtx other; - other = mips_pic_call_symbol_from_set (defs->ref, reg); + other = mips_pic_call_symbol_from_set (defs->ref, reg, recurse_p); if (!rtx_equal_p (symbol, other)) return NULL_RTX; } @@ -14164,7 +14186,7 @@ mips_annotate_pic_calls (void) if (!REG_P (reg)) continue; - symbol = mips_find_pic_call_symbol (insn, reg); + symbol = mips_find_pic_call_symbol (insn, reg, true); if (symbol) { mips_annotate_pic_call_expr (call, symbol);