From patchwork Fri May 24 00:43:32 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joern Rennecke X-Patchwork-Id: 246042 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 3A5112C008A for ; Fri, 24 May 2013 10:43:43 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:to:cc:subject:references:in-reply-to :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=HT+UCAWF8XmXpYMhkGfMTn3J62tW0KUBmPvZdUJUJbikmg+QPh/In 75+BHQSaPQniuU2Nd+GKixmPbF6cZL629vERGGuPc96XUOvujj77VzmivDr+G2jw sN015/0CMVZzCwRpWLWzSv/b3FsjDNKo4ONetRkfuDXN/81FAxM6tM= 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 :message-id:date:from:to:cc:subject:references:in-reply-to :mime-version:content-type:content-transfer-encoding; s=default; bh=xE05Yq6p50L4yU9BVR7utK2nbKs=; b=koa5ErOwAWhZx5Ml1TL6Iw4KEJ7u YJ4yt5RckJqprNKdtxB5EzyACQPrJ4V34A5dEdPkPE1mMBwNOy+vdGEncBIvsrpS LXvmsiU84lk7bjrYGX80HkHqqlVel83eNpVe1pJudlVWtSQFB366jxj9ZgXG8d95 kb5HqOU/QCRY4DQ= Received: (qmail 32529 invoked by alias); 24 May 2013 00:43:36 -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 32520 invoked by uid 89); 24 May 2013 00:43:36 -0000 X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, MIME_QP_LONG_LINE, RCVD_IN_DNSWL_MED, TW_DB autolearn=ham version=3.3.1 Received: from c62.cesmail.net (HELO c62.cesmail.net) (216.154.195.54) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 24 May 2013 00:43:34 +0000 Received: from unknown (HELO delta2) ([192.168.1.50]) by c62.cesmail.net with ESMTP; 23 May 2013 20:43:32 -0400 Received: from cust213-dsl91-135-11.idnet.net (cust213-dsl91-135-11.idnet.net [91.135.11.213]) by webmail.spamcop.net (Horde MIME library) with HTTP; Thu, 23 May 2013 20:43:32 -0400 Message-ID: <20130523204332.0fgv95tiskws000g-nzlynne@webmail.spamcop.net> Date: Thu, 23 May 2013 20:43:32 -0400 From: Joern Rennecke To: Alexandre Oliva Cc: Eric Botcazou , gcc-patches@gcc.gnu.org, Kaz Kojima Subject: SUBREGs in move2add_note_store (Was: Re: RFA: fix rtl-optimization/56833) References: <20130515110002.j5o6zwvv8c88c48g-nzlynne@webmail.spamcop.net> <1465225.euvHJY1BXk@polaris> <20130522064532.lk8zcp0dkowc8o4w-nzlynne@webmail.spamcop.net> <2178592.7Mt0ckje7d@polaris> <20130523163350.u0nucgcuroo4kgk0-nzlynne@webmail.spamcop.net> In-Reply-To: <20130523163350.u0nucgcuroo4kgk0-nzlynne@webmail.spamcop.net> MIME-Version: 1.0 User-Agent: Internet Messaging Program (IMP) H3 (4.1.4) X-Virus-Found: No Quoting Joern Rennecke : > Looking into sharing the code with sites that perform essentially the same > function but look somewhat different, I see there's a problem with using > only reg_set_luid to indicate the consistency of a multi-hard-reg-value > in these other contexts. > For values that are use a base register, the reg_set_luid is the same as > for the base register; for constants, it is the same for all constants > set since the last label. > > Say we have reg size 8 bit, base r0, and then > (set reg:HI 2 (plus:SI (reg:HI 0) (const_int 500)) > ... > (set reg:HI 3 (plus:SI (reg:HI 0) (const_int 500)) > > Now how do we tell that the value in r2 is no longer valid? > > As the example shows, trying to replicate the recorded value across all hard > regs is pointless, as we still need to make sure that we have a still-valid > start register. > OTOH, this ties in nicely with setting the mode of subsequent registers > to VOIDmode. We can verify the mode to make sure there was no more recent > set of any constituent register. The check of the extra luids thus becomes > superfluous, as becomes the set. I've found it is good to have also one mode to invalidate a register for all uses; it seems natural to use VOIDmode for that, and then we can use BLKmode for all but the first hard register of a multi-hard-reg register. However, digging into the code that can use some factoring out of mode setting and validity testing, I stumbled over the destination handling in move2add_note_store in the case of SUBREGs. The original code was just coded or minimal effort to find the actual regno of a REG or SUBREG (this only made sense with the old word-based SUBREG scheme). Alexandre, AFAICT, you have misunderstood this bit to make the function pretend that the inner part of the subreg is the actual destination. Or is there some obscure reason to have the mode checks worry about the inside of the SUBREG? I have attached the patch that makes the reload_cse_move2add sub-pass work the way it think it should be; it'll take some time to test this properly, though. 2013-05-24 Joern Rennecke * postreload.c (move2add_record_mode): New function. (move2add_record_sym_value, move2add_valid_value_p): Likewise. (move2add_use_add2_insn): Use move2add_record_sym_value. (move2add_use_add3_insn): Likewise. (reload_cse_move2add): Use move2add_valid_value_p and move2add_record_mode. Invalidate call-clobbered regs by setting reg_mode to VOIDmode. (move2add_note_store): Don't pretend the inside of a SUBREG is the actual destination. Invalidate PRE_INC / POST_INC registers my setting reg_mode to VOIDmode. Use move2add_record_sym_value, move2add_valid_value_p and move2add_record_mode. Index: postreload.c =================================================================== --- postreload.c (revision 199190) +++ postreload.c (working copy) @@ -1645,14 +1645,22 @@ reload_combine_note_use (rtx *xp, rtx in later disable any optimization that would cross it. reg_offset[n] / reg_base_reg[n] / reg_symbol_ref[n] / reg_mode[n] are only valid if reg_set_luid[n] is greater than - move2add_last_label_luid. */ + move2add_last_label_luid. + For a set that established a new (potential) base register with + non-constant value, we use move2add_luid from the place where the + setting insn is encountered; registers based off that base then + get the same reg_set_luid. Constants all get + move2add_last_label_luid + 1 as their reg_set_luid. */ static int reg_set_luid[FIRST_PSEUDO_REGISTER]; /* If reg_base_reg[n] is negative, register n has been set to reg_offset[n] or reg_symbol_ref[n] + reg_offset[n] in mode reg_mode[n]. If reg_base_reg[n] is non-negative, register n has been set to the sum of reg_offset[n] and the value of register reg_base_reg[n] - before reg_set_luid[n], calculated in mode reg_mode[n] . */ + before reg_set_luid[n], calculated in mode reg_mode[n] . + For multi-hard-register registers, all but the first one are + recorded as BLKmode in reg_mode. Setting reg_mode to VOIDmode + marks it as invalid. */ static HOST_WIDE_INT reg_offset[FIRST_PSEUDO_REGISTER]; static int reg_base_reg[FIRST_PSEUDO_REGISTER]; static rtx reg_symbol_ref[FIRST_PSEUDO_REGISTER]; @@ -1674,6 +1682,63 @@ #define MODES_OK_FOR_MOVE2ADD(OUTMODE, I || (GET_MODE_SIZE (OUTMODE) <= GET_MODE_SIZE (INMODE) \ && TRULY_NOOP_TRUNCATION_MODES_P (OUTMODE, INMODE))) +/* Record that REG is being set to a value with the mode of REG. */ + +static void +move2add_record_mode (rtx reg) +{ + int regno, nregs; + enum machine_mode mode = GET_MODE (reg); + int i; + + if (GET_CODE (reg) == SUBREG) + { + regno = subreg_regno (reg); + nregs = subreg_nregs (reg); + } + else if (REG_P (reg)) + { + regno = REGNO (reg); + nregs = hard_regno_nregs[regno][mode]; + } + else + gcc_unreachable (); + for (i = nregs; --i > 0; ) + reg_mode[regno + i] = BLKmode; + reg_mode[regno] = mode; +} + +/* Record that REG is being set to the sum of SYM and OFF. */ + +static void +move2add_record_sym_value (rtx reg, rtx sym, rtx off) +{ + int regno = REGNO (reg); + + move2add_record_mode (reg); + reg_set_luid[regno] = move2add_luid; + reg_base_reg[regno] = -1; + reg_symbol_ref[regno] = sym; + reg_offset[regno] = INTVAL (off); +} + +/* Check if REGNO contains a valid value in MODE. */ + +static bool +move2add_valid_value_p (int regno, enum machine_mode mode) +{ + int i; + + if (reg_set_luid[regno] <= move2add_last_label_luid + || !MODES_OK_FOR_MOVE2ADD (mode, reg_mode[regno])) + return false; + + for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--) + if (reg_mode[i] != BLKmode) + return false; + return true; +} + /* This function is called with INSN that sets REG to (SYM + OFF), while REG is known to already have value (SYM + offset). This function tries to change INSN into an add instruction @@ -1749,11 +1814,7 @@ move2add_use_add2_insn (rtx reg, rtx sym } } } - reg_set_luid[regno] = move2add_luid; - reg_base_reg[regno] = -1; - reg_mode[regno] = GET_MODE (reg); - reg_symbol_ref[regno] = sym; - reg_offset[regno] = INTVAL (off); + move2add_record_sym_value (reg, sym, off); return changed; } @@ -1787,8 +1848,7 @@ move2add_use_add3_insn (rtx reg, rtx sym SET_SRC (pat) = plus_expr; for (i = 0; i < FIRST_PSEUDO_REGISTER; i++) - if (reg_set_luid[i] > move2add_last_label_luid - && reg_mode[i] == GET_MODE (reg) + if (move2add_valid_value_p (i, GET_MODE (reg)) && reg_base_reg[i] < 0 && reg_symbol_ref[i] != NULL_RTX && rtx_equal_p (sym, reg_symbol_ref[i])) @@ -1836,10 +1896,7 @@ move2add_use_add3_insn (rtx reg, rtx sym changed = true; } reg_set_luid[regno] = move2add_luid; - reg_base_reg[regno] = -1; - reg_mode[regno] = GET_MODE (reg); - reg_symbol_ref[regno] = sym; - reg_offset[regno] = INTVAL (off); + move2add_record_sym_value (reg, sym, off); return changed; } @@ -1890,8 +1947,7 @@ reload_cse_move2add (rtx first) /* Check if we have valid information on the contents of this register in the mode of REG. */ - if (reg_set_luid[regno] > move2add_last_label_luid - && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno]) + if (move2add_valid_value_p (regno, GET_MODE (reg)) && dbg_cnt (cse2_move2add)) { /* Try to transform (set (REGX) (CONST_INT A)) @@ -1928,8 +1984,7 @@ reload_cse_move2add (rtx first) else if (REG_P (src) && reg_set_luid[regno] == reg_set_luid[REGNO (src)] && reg_base_reg[regno] == reg_base_reg[REGNO (src)] - && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), - reg_mode[REGNO (src)])) + && move2add_valid_value_p (REGNO (src), GET_MODE (reg))) { rtx next = next_nonnote_nondebug_insn (insn); rtx set = NULL_RTX; @@ -1982,10 +2037,10 @@ reload_cse_move2add (rtx first) delete_insn (insn); changed |= success; insn = next; - reg_mode[regno] = GET_MODE (reg); - reg_offset[regno] = - trunc_int_for_mode (added_offset + base_offset, - GET_MODE (reg)); + move2add_record_mode (reg); + reg_offset[regno] + = trunc_int_for_mode (added_offset + base_offset, + GET_MODE (reg)); continue; } } @@ -2021,8 +2076,7 @@ reload_cse_move2add (rtx first) /* If the reg already contains the value which is sum of sym and some constant value, we can use an add2 insn. */ - if (reg_set_luid[regno] > move2add_last_label_luid - && MODES_OK_FOR_MOVE2ADD (GET_MODE (reg), reg_mode[regno]) + if (move2add_valid_value_p (regno, GET_MODE (reg)) && reg_base_reg[regno] < 0 && reg_symbol_ref[regno] != NULL_RTX && rtx_equal_p (sym, reg_symbol_ref[regno])) @@ -2045,7 +2099,10 @@ reload_cse_move2add (rtx first) /* Reset the information about this register. */ int regno = REGNO (XEXP (note, 0)); if (regno < FIRST_PSEUDO_REGISTER) - reg_set_luid[regno] = 0; + { + move2add_record_mode (XEXP (note, 0)); + reg_set_luid[regno] = 0; + } } } note_stores (PATTERN (insn), move2add_note_store, insn); @@ -2082,7 +2139,7 @@ reload_cse_move2add (rtx first) { if (call_used_regs[i]) /* Reset the information about this register. */ - reg_set_luid[i] = 0; + reg_mode[i] = VOIDmode; } } } @@ -2099,20 +2156,8 @@ move2add_note_store (rtx dst, const_rtx { rtx insn = (rtx) data; unsigned int regno = 0; - unsigned int nregs = 0; - unsigned int i; enum machine_mode mode = GET_MODE (dst); - if (GET_CODE (dst) == SUBREG) - { - regno = subreg_regno_offset (REGNO (SUBREG_REG (dst)), - GET_MODE (SUBREG_REG (dst)), - SUBREG_BYTE (dst), - GET_MODE (dst)); - nregs = subreg_nregs (dst); - dst = SUBREG_REG (dst); - } - /* Some targets do argument pushes without adding REG_INC notes. */ if (MEM_P (dst)) @@ -2120,27 +2165,28 @@ move2add_note_store (rtx dst, const_rtx dst = XEXP (dst, 0); if (GET_CODE (dst) == PRE_INC || GET_CODE (dst) == POST_INC || GET_CODE (dst) == PRE_DEC || GET_CODE (dst) == POST_DEC) - reg_set_luid[REGNO (XEXP (dst, 0))] = 0; + reg_mode[REGNO (XEXP (dst, 0))] = VOIDmode; return; } - if (!REG_P (dst)) - return; - regno += REGNO (dst); - if (!nregs) - nregs = hard_regno_nregs[regno][mode]; + if (GET_CODE (dst) == SUBREG) + regno = subreg_regno (dst); + else if (REG_P (dst)) + regno = REGNO (dst); + else + return; - if (SCALAR_INT_MODE_P (GET_MODE (dst)) - && nregs == 1 && GET_CODE (set) == SET) + if (SCALAR_INT_MODE_P (mode) + && GET_CODE (set) == SET) { rtx note, sym = NULL_RTX; - HOST_WIDE_INT off; + rtx off; note = find_reg_equal_equiv_note (insn); if (note && GET_CODE (XEXP (note, 0)) == SYMBOL_REF) { sym = XEXP (note, 0); - off = 0; + off = const0_rtx; } else if (note && GET_CODE (XEXP (note, 0)) == CONST && GET_CODE (XEXP (XEXP (note, 0), 0)) == PLUS @@ -2148,22 +2194,18 @@ move2add_note_store (rtx dst, const_rtx && CONST_INT_P (XEXP (XEXP (XEXP (note, 0), 0), 1))) { sym = XEXP (XEXP (XEXP (note, 0), 0), 0); - off = INTVAL (XEXP (XEXP (XEXP (note, 0), 0), 1)); + off = XEXP (XEXP (XEXP (note, 0), 0), 1); } if (sym != NULL_RTX) { - reg_base_reg[regno] = -1; - reg_symbol_ref[regno] = sym; - reg_offset[regno] = off; - reg_mode[regno] = mode; - reg_set_luid[regno] = move2add_luid; + move2add_record_sym_value (dst, sym, off); return; } } - if (SCALAR_INT_MODE_P (GET_MODE (dst)) - && nregs == 1 && GET_CODE (set) == SET + if (SCALAR_INT_MODE_P (mode) + && GET_CODE (set) == SET && GET_CODE (SET_DEST (set)) != ZERO_EXTRACT && GET_CODE (SET_DEST (set)) != STRICT_LOW_PART) { @@ -2171,9 +2213,6 @@ move2add_note_store (rtx dst, const_rtx rtx base_reg; HOST_WIDE_INT offset; int base_regno; - /* This may be different from mode, if SET_DEST (set) is a - SUBREG. */ - enum machine_mode dst_mode = GET_MODE (dst); switch (GET_CODE (src)) { @@ -2185,20 +2224,14 @@ move2add_note_store (rtx dst, const_rtx if (CONST_INT_P (XEXP (src, 1))) offset = INTVAL (XEXP (src, 1)); else if (REG_P (XEXP (src, 1)) - && (reg_set_luid[REGNO (XEXP (src, 1))] - > move2add_last_label_luid) - && (MODES_OK_FOR_MOVE2ADD - (dst_mode, reg_mode[REGNO (XEXP (src, 1))]))) + && move2add_valid_value_p (REGNO (XEXP (src, 1)), mode)) { if (reg_base_reg[REGNO (XEXP (src, 1))] < 0 && reg_symbol_ref[REGNO (XEXP (src, 1))] == NULL_RTX) offset = reg_offset[REGNO (XEXP (src, 1))]; /* Maybe the first register is known to be a constant. */ - else if (reg_set_luid[REGNO (base_reg)] - > move2add_last_label_luid - && (MODES_OK_FOR_MOVE2ADD - (dst_mode, reg_mode[REGNO (base_reg)])) + else if (move2add_valid_value_p (REGNO (base_reg), mode) && reg_base_reg[REGNO (base_reg)] < 0 && reg_symbol_ref[REGNO (base_reg)] == NULL_RTX) { @@ -2228,33 +2261,26 @@ move2add_note_store (rtx dst, const_rtx reg_offset[regno] = INTVAL (SET_SRC (set)); /* We assign the same luid to all registers set to constants. */ reg_set_luid[regno] = move2add_last_label_luid + 1; - reg_mode[regno] = mode; + move2add_record_mode (dst); return; default: - invalidate: - /* Invalidate the contents of the register. */ - reg_set_luid[regno] = 0; - return; + goto invalidate; } base_regno = REGNO (base_reg); /* If information about the base register is not valid, set it up as a new base register, pretending its value is known starting from the current insn. */ - if (reg_set_luid[base_regno] <= move2add_last_label_luid) + if (!move2add_valid_value_p (base_regno, mode)) { reg_base_reg[base_regno] = base_regno; reg_symbol_ref[base_regno] = NULL_RTX; reg_offset[base_regno] = 0; reg_set_luid[base_regno] = move2add_luid; - reg_mode[base_regno] = mode; + gcc_assert (GET_MODE (base_reg) == mode); + move2add_record_mode (base_reg); } - else if (! MODES_OK_FOR_MOVE2ADD (dst_mode, - reg_mode[base_regno])) - goto invalidate; - - reg_mode[regno] = mode; /* Copy base information from our base register. */ reg_set_luid[regno] = reg_set_luid[base_regno]; @@ -2262,17 +2288,17 @@ move2add_note_store (rtx dst, const_rtx reg_symbol_ref[regno] = reg_symbol_ref[base_regno]; /* Compute the sum of the offsets or constants. */ - reg_offset[regno] = trunc_int_for_mode (offset - + reg_offset[base_regno], - dst_mode); + reg_offset[regno] + = trunc_int_for_mode (offset + reg_offset[base_regno], mode); + + move2add_record_mode (dst); } else { - unsigned int endregno = regno + nregs; - - for (i = regno; i < endregno; i++) - /* Reset the information about this register. */ - reg_set_luid[i] = 0; + invalidate: + /* Invalidate the contents of the register. */ + reg_set_luid[regno] = 0; + move2add_record_mode (dst); } }