From patchwork Thu Apr 4 20:53:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 1077721 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-498860-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="f5JFBdO/"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44ZwD15ppJz9sPC for ; Fri, 5 Apr 2019 07:54:08 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:message-id:date:mime-version:content-type; q=dns; s= default; b=wXC/juzCpTljft7oJL7QXO6uwgjqW+TA2aOf2npwFDQEsCYsOTKb+ M1SO8jVdwCqXEPsuowWxVLbs/pcpLbBol3eG446O+bBJIhQKeUL2o8ygBI2blPNd ORjutn+G4D3sUDwQu34CgjjqzsUy/NgfcZlwUbC69XzphtekLKy6NE= 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:from :to:subject:message-id:date:mime-version:content-type; s= default; bh=GjEPTPn20PRj4AeMn9Fnsoov4yI=; b=f5JFBdO/jpibNsh7GovN fOYT58RrHF5dpbyjfVM7PPJJQ0/1buUp+uivA79rQV7joEqLBYqOOR0TPPyLpBi8 Tb3NS8NUtQBEkZFUjJphKOiA/UV79jY6vxKqc5bEkXkMdetNiS7yUa9Ild/81It7 2fRHxUDM/GLzfUCwuWDHtCg= Received: (qmail 103163 invoked by alias); 4 Apr 2019 20:54:01 -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 103155 invoked by uid 89); 4 Apr 2019 20:54:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=sweep X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 04 Apr 2019 20:53:59 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 09EA259447 for ; Thu, 4 Apr 2019 20:53:58 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-71.rdu2.redhat.com [10.10.112.71]) by smtp.corp.redhat.com (Postfix) with ESMTP id 24E5A19C65 for ; Thu, 4 Apr 2019 20:53:56 +0000 (UTC) From: Jeff Law Openpgp: preference=signencrypt To: gcc-patches Subject: [committed][PR rtl-optimization/89399] Safely access the source/destination of sets Message-ID: Date: Thu, 4 Apr 2019 14:53:55 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 X-IsSubscribed: yes As noted in the BZ ree was incorrectly looking at SET_SRC/SET_DEST of PATTERN of various insns. In the case where we're looking at something in the candidate list, we know that all the insns passed the single_set test. So when digging into something from the candidate list, we should call single_set to extract the set, then use SET_SRC/SET_DEST to look at the appropriate parts of the set. That's precisely what this patch does. Other places within ree.c use get_sub_rtx, but that seems to be for the insn we want to change rather than items on the candidate list. get_sub_rtx uses PATTERN (insn) internally, but does so in a reasonably safe manner. transform_ifelse also uses PATTERN (insn), but AFAICT only does so for conditional moves which passed the is_cond_copy_insn test which includes a single_set test. So those are safe as well. Bootstrapped and regression tested on a variety of platforms. Installing on the trunk. Jeff commit 289d708e113e0d579eb78f9b5e63fb6146288cf5 Author: Jeff Law Date: Thu Apr 4 14:49:53 2019 -0600 PR rtl-optimization/89399 * ree.c (combine_set_extension): Use single_set rather than digging into PATTERN for items on the candidate list. (combine_reaching_defs): Likewise. PR rtl-optimization/89399 * gcc.c-torture/compile/pr89399.c: New test. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 47eeed65ab7..f57c2f9c193 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2019-04-04 Jeff Law + + PR rtl-optimization/89399 + * ree.c (combine_set_extension): Use single_set rather than + digging into PATTERN for items on the candidate list. + (combine_reaching_defs): Likewise. + 2019-04-04 Richard Sandiford PR rtl-optimization/46590 diff --git a/gcc/ree.c b/gcc/ree.c index e23e607a5e1..104f8dbf435 100644 --- a/gcc/ree.c +++ b/gcc/ree.c @@ -320,7 +320,7 @@ combine_set_extension (ext_cand *cand, rtx_insn *curr_insn, rtx *orig_set) rtx orig_src = SET_SRC (*orig_set); machine_mode orig_mode = GET_MODE (SET_DEST (*orig_set)); rtx new_set; - rtx cand_pat = PATTERN (cand->insn); + rtx cand_pat = single_set (cand->insn); /* If the extension's source/destination registers are not the same then we need to change the original load to reference the destination @@ -778,10 +778,14 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) /* If the destination operand of the extension is a different register than the source operand, then additional restrictions are needed. Note we have to handle cases where we have nested - extensions in the source operand. */ + extensions in the source operand. + + Candidate insns are known to be single_sets, via the test in + find_removable_extensions. So we continue to use single_set here + rather than get_sub_rtx. */ + rtx set = single_set (cand->insn); bool copy_needed - = (REGNO (SET_DEST (PATTERN (cand->insn))) - != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn))))); + = (REGNO (SET_DEST (set)) != REGNO (get_extended_src_reg (SET_SRC (set)))); if (copy_needed) { /* Considering transformation of @@ -816,8 +820,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE) return false; - machine_mode dst_mode = GET_MODE (SET_DEST (PATTERN (cand->insn))); - rtx src_reg = get_extended_src_reg (SET_SRC (PATTERN (cand->insn))); + machine_mode dst_mode = GET_MODE (SET_DEST (set)); + rtx src_reg = get_extended_src_reg (SET_SRC (set)); /* Ensure we can use the src_reg in dst_mode (needed for the (set (reg1) (reg2)) insn mentioned above). */ @@ -852,9 +856,9 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) || !REG_P (SET_DEST (*dest_sub_rtx))) return false; - rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (PATTERN (cand->insn))), + rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (set)), REGNO (SET_DEST (*dest_sub_rtx))); - if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (PATTERN (cand->insn)))) + if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (set))) return false; /* On RISC machines we must make sure that changing the mode of SRC_REG @@ -877,10 +881,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) /* The destination register of the extension insn must not be used or set between the def_insn and cand->insn exclusive. */ - if (reg_used_between_p (SET_DEST (PATTERN (cand->insn)), - def_insn, cand->insn) - || reg_set_between_p (SET_DEST (PATTERN (cand->insn)), - def_insn, cand->insn)) + if (reg_used_between_p (SET_DEST (set), def_insn, cand->insn) + || reg_set_between_p (SET_DEST (set), def_insn, cand->insn)) return false; /* We must be able to copy between the two registers. Generate, @@ -894,11 +896,10 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) is different than in the code to emit the copy as we have not modified the defining insn yet. */ start_sequence (); - rtx pat = PATTERN (cand->insn); - rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (pat)), - REGNO (get_extended_src_reg (SET_SRC (pat)))); - rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (pat)), - REGNO (SET_DEST (pat))); + rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (set)), + REGNO (get_extended_src_reg (SET_SRC (set)))); + rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (set)), + REGNO (SET_DEST (set))); emit_move_insn (new_dst, new_src); rtx_insn *insn = get_insns (); @@ -912,7 +913,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) return false; while (REG_P (SET_SRC (*dest_sub_rtx)) - && (REGNO (SET_SRC (*dest_sub_rtx)) == REGNO (SET_DEST (pat)))) + && (REGNO (SET_SRC (*dest_sub_rtx)) == REGNO (SET_DEST (set)))) { /* Considering transformation of (set (reg2) (expression)) @@ -955,7 +956,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) implicitly sets it in word mode. */ if (WORD_REGISTER_OPERATIONS && known_lt (prec, BITS_PER_WORD)) { - struct df_link *uses = get_uses (def_insn2, SET_DEST (pat)); + struct df_link *uses = get_uses (def_insn2, SET_DEST (set)); if (!uses) break; @@ -973,10 +974,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) used or set between the def_insn2 and def_insn exclusive. Likewise for the other reg, i.e. check both reg1 and reg2 in the above comment. */ - if (reg_used_between_p (SET_DEST (PATTERN (cand->insn)), - def_insn2, def_insn) - || reg_set_between_p (SET_DEST (PATTERN (cand->insn)), - def_insn2, def_insn) + if (reg_used_between_p (SET_DEST (set), def_insn2, def_insn) + || reg_set_between_p (SET_DEST (set), def_insn2, def_insn) || reg_used_between_p (src_reg, def_insn2, def_insn) || reg_set_between_p (src_reg, def_insn2, def_insn)) break; @@ -991,13 +990,12 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state) if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE) { machine_mode mode; - rtx set; if (state->modified[INSN_UID (cand->insn)].kind != (cand->code == ZERO_EXTEND ? EXT_MODIFIED_ZEXT : EXT_MODIFIED_SEXT) || state->modified[INSN_UID (cand->insn)].mode != cand->mode - || (set = single_set (cand->insn)) == NULL_RTX) + || (set == NULL_RTX)) return false; mode = GET_MODE (SET_DEST (set)); gcc_assert (GET_MODE_UNIT_SIZE (mode) @@ -1320,9 +1318,9 @@ find_and_remove_re (void) we do not allow the optimization of extensions where the source and destination registers do not match. Thus checking REG_P here is correct. */ - if (REG_P (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0)) - && (REGNO (SET_DEST (PATTERN (curr_cand->insn))) - != REGNO (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0)))) + rtx set = single_set (curr_cand->insn); + if (REG_P (XEXP (SET_SRC (set), 0)) + && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0)))) { reinsn_copy_list.safe_push (curr_cand->insn); reinsn_copy_list.safe_push (state.defs_list[0]); @@ -1356,13 +1354,13 @@ find_and_remove_re (void) defining insn was used to eliminate a second extension that was wider than the first. */ rtx sub_rtx = *get_sub_rtx (def_insn); - rtx pat = PATTERN (curr_insn); + rtx set = single_set (curr_insn); rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)), - REGNO (XEXP (SET_SRC (pat), 0))); + REGNO (XEXP (SET_SRC (set), 0))); rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)), - REGNO (SET_DEST (pat))); - rtx set = gen_rtx_SET (new_dst, new_src); - emit_insn_after (set, def_insn); + REGNO (SET_DEST (set))); + rtx new_set = gen_rtx_SET (new_dst, new_src); + emit_insn_after (new_set, def_insn); } /* Delete all useless extensions here in one sweep. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 7afa590a8ef..910d55b56f5 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2019-04-04 Jeff Law + + PR rtl-optimization/89399 + * gcc.c-torture/compile/pr89399.c: New test. + 2019-04-04 Paolo Carlini PR c++/65619 diff --git a/gcc/testsuite/gcc.c-torture/compile/pr89399.c b/gcc/testsuite/gcc.c-torture/compile/pr89399.c new file mode 100644 index 00000000000..ce8fbfc4fb8 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr89399.c @@ -0,0 +1,10 @@ +extern int bar(void); + +short s; + +int foo(void) +{ + s = bar(); + return s; +} +