From patchwork Tue Apr 24 22:06:12 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 154768 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 98AB1B708A for ; Wed, 25 Apr 2012 08:06:34 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1335909995; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Mail-Followup-To:Cc:Subject:References:Date: In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=yZL4n4qmPDg2r6F9VJSs 4wrXd9Y=; b=JzfqmBhNhwj6qoWFIl+BqTnQKVahzVWNTg3nGx9rHjhOxhlYiiyb z8lgPXtX0QbNr55+4FvODXo/QBgr2KcKj7V8VHnWl3q93PDJQWWXOrXdP6hv51bJ d69JyywtAWCV9yxObA3/nMSdERSAVpuQarIIW147Xp8WBBZFCowEj8U= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:From:To:Mail-Followup-To:Cc:Subject:References:Date:In-Reply-To:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=nOp0h8aqnB0G5s/DAqA0sg7QTamkNycbAGawlIfcC7o4f8CFdcswVmbCfiBu8P LWlK3SnaWu2NIzCO8NRlAFleWvfNR8YA02CRzcY2WbRjsgj8EO/zow5mtJ7pLD/Z 0jPcpVpgpUGksEm63vKcPsaF7gkYrSLIwxS270AQ+mX7c=; Received: (qmail 26703 invoked by alias); 24 Apr 2012 22:06:30 -0000 Received: (qmail 26695 invoked by uid 22791); 24 Apr 2012 22:06:29 -0000 X-SWARE-Spam-Status: No, hits=-4.1 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-wi0-f169.google.com (HELO mail-wi0-f169.google.com) (209.85.212.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 Apr 2012 22:06:15 +0000 Received: by wibhm17 with SMTP id hm17so4407568wib.2 for ; Tue, 24 Apr 2012 15:06:14 -0700 (PDT) Received: by 10.180.104.137 with SMTP id ge9mr475369wib.20.1335305174226; Tue, 24 Apr 2012 15:06:14 -0700 (PDT) Received: from localhost (rsandifo.gotadsl.co.uk. [82.133.89.107]) by mx.google.com with ESMTPS id ex2sm51145039wib.8.2012.04.24.15.06.12 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 24 Apr 2012 15:06:13 -0700 (PDT) From: Richard Sandiford To: Olivier Hainque Mail-Followup-To: Olivier Hainque , GCC Patches , rdsandiford@googlemail.com Cc: GCC Patches Subject: Re: strengthen protection against REG_EQUIV/EQUAL on !REG set References: <9DFC92BF-613B-4A48-ABE8-794147DFCEAF@adacore.com> Date: Tue, 24 Apr 2012 23:06:12 +0100 In-Reply-To: <9DFC92BF-613B-4A48-ABE8-794147DFCEAF@adacore.com> (Olivier Hainque's message of "Thu, 12 Apr 2012 11:54:43 +0200") Message-ID: <871unc7pkb.fsf@talisman.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (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 Olivier Hainque writes: > *** /tmp/rkQ7Ep_emit-rtl.c 2012-04-12 11:19:51.830104512 +0200 > --- gcc/emit-rtl.c 2012-04-11 22:39:11.323103686 +0200 > *************** set_unique_reg_note (rtx insn, enum reg_ > *** 4955,4960 **** > --- 4955,4975 ---- > if (GET_CODE (datum) == ASM_OPERANDS) > return NULL_RTX; > > + /* Don't add REG_EQUAL/REG_EQUIV notes on a single_set destination which > + isn't a REG or a SUBREG of REG. Such notes are invalid, could lead > + to bogus assumptions downstream (e.g. that a (set MEM) couldn't trap), > + and many callers just don't care checking. Note that we might have > + single_set (insn) == 0 here, typically from reload attaching REG_EQUAL > + to USEs for inheritance processing purposes. */ > + { > + rtx set = single_set (insn); > + > + if (set && ! (REG_P (SET_DEST (set)) > + || (GET_CODE (SET_DEST (set)) == SUBREG > + && REG_P (SUBREG_REG (SET_DEST (set)))))) > + return NULL_RTX; > + } > + STRICT_LOW_PART is OK too. I like the idea, but I think we're in danger of having too many functions check the set. Further up set_unique_reg_note we have: /* Don't add REG_EQUAL/REG_EQUIV notes if the insn has multiple sets (some callers assume single_set means the insn only has one set, when in fact it means the insn only has one * useful * set). */ if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets (insn)) { gcc_assert (!note); return NULL_RTX; } And set_dst_reg_note has: rtx set = single_set (insn); if (set && SET_DEST (set) == dst) return set_unique_reg_note (insn, kind, datum); Would be nice to use a single function that knows about the extra contraints here. Maybe something like the attached? I'm deliberately requiring the SET to the first rtx in the PARALLEL. I'm not entirely happy with the optabs.c code: if (! rtx_equal_p (SET_DEST (set), target) /* For a STRICT_LOW_PART, the REG_NOTE applies to what is inside it. */ && (GET_CODE (SET_DEST (set)) != STRICT_LOW_PART || ! rtx_equal_p (XEXP (SET_DEST (set), 0), target))) return 1; either; the note is always GET_MODE (target), so surely this should be checking for STRICT_LOW_PART before applying rtx_equal_p? Maybe set_for_reg_notes should return the reg too, and we'd just apply rtx_equal_p to that. Richard gcc/ * rtl.h (set_for_reg_notes): Declare. * emit-rtl.c (set_for_reg_notes): New function. (set_unique_reg_note): Use it. * optabs.c (add_equal_note): Likewise. Index: gcc/rtl.h =================================================================== --- gcc/rtl.h 2012-04-24 22:55:36.002967164 +0100 +++ gcc/rtl.h 2012-04-24 22:59:34.150966581 +0100 @@ -1879,6 +1879,7 @@ extern enum machine_mode choose_hard_reg bool); /* In emit-rtl.c */ +extern rtx set_for_reg_notes (rtx); extern rtx set_unique_reg_note (rtx, enum reg_note, rtx); extern rtx set_dst_reg_note (rtx, enum reg_note, rtx, rtx); extern void set_insn_deleted (rtx); Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c 2012-04-24 22:55:36.002967164 +0100 +++ gcc/emit-rtl.c 2012-04-24 23:00:13.677966484 +0100 @@ -4944,6 +4944,45 @@ force_next_line_note (void) last_location = -1; } +/* Notes like REG_EQUAL and REG_EQUIV refer to a set in an instruction. + Return the set in INSN that such notes describe, or NULL if the notes + have no meaning for INSN. */ + +rtx +set_for_reg_notes (rtx insn) +{ + rtx pat, reg; + + if (!INSN_P (insn)) + return NULL_RTX; + + pat = PATTERN (insn); + if (GET_CODE (pat) == PARALLEL) + { + /* We do not use single_set because that ignores SETs of unused + registers. REG_EQUAL and REG_EQUIV notes really do require the + PARALLEL to have a single SET. */ + if (multiple_sets (insn)) + return NULL_RTX; + pat = XVECEXP (pat, 0, 0); + } + + if (GET_CODE (pat) != SET) + return NULL_RTX; + + reg = SET_DEST (pat); + + /* Notes apply to the contents of a STRICT_LOW_PART. */ + if (GET_CODE (reg) == STRICT_LOW_PART) + reg = XEXP (reg, 0); + + /* Check that we have a register. */ + if (!(REG_P (reg) || GET_CODE (reg) == SUBREG)) + return NULL_RTX; + + return pat; +} + /* Place a note of KIND on insn INSN with DATUM as the datum. If a note of this type already exists, remove it first. */ @@ -4956,39 +4995,26 @@ set_unique_reg_note (rtx insn, enum reg_ { case REG_EQUAL: case REG_EQUIV: - /* Don't add REG_EQUAL/REG_EQUIV notes if the insn - has multiple sets (some callers assume single_set - means the insn only has one set, when in fact it - means the insn only has one * useful * set). */ - if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets (insn)) - { - gcc_assert (!note); - return NULL_RTX; - } + if (!set_for_reg_notes (insn)) + return NULL_RTX; /* Don't add ASM_OPERAND REG_EQUAL/REG_EQUIV notes. It serves no useful purpose and breaks eliminate_regs. */ if (GET_CODE (datum) == ASM_OPERANDS) return NULL_RTX; - - if (note) - { - XEXP (note, 0) = datum; - df_notes_rescan (insn); - return note; - } break; default: - if (note) - { - XEXP (note, 0) = datum; - return note; - } break; } - add_reg_note (insn, kind, datum); + if (note) + XEXP (note, 0) = datum; + else + { + add_reg_note (insn, kind, datum); + note = REG_NOTES (insn); + } switch (kind) { @@ -5000,14 +5026,14 @@ set_unique_reg_note (rtx insn, enum reg_ break; } - return REG_NOTES (insn); + return note; } /* Like set_unique_reg_note, but don't do anything unless INSN sets DST. */ rtx set_dst_reg_note (rtx insn, enum reg_note kind, rtx datum, rtx dst) { - rtx set = single_set (insn); + rtx set = set_for_reg_notes (insn); if (set && SET_DEST (set) == dst) return set_unique_reg_note (insn, kind, datum); Index: gcc/optabs.c =================================================================== --- gcc/optabs.c 2012-04-24 22:55:36.002967164 +0100 +++ gcc/optabs.c 2012-04-24 22:59:34.148966581 +0100 @@ -191,7 +191,7 @@ add_equal_note (rtx insns, rtx target, e last_insn = NEXT_INSN (last_insn)) ; - set = single_set (last_insn); + set = set_for_reg_notes (last_insn); if (set == NULL_RTX) return 1;