From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117056 invoked by alias); 29 Sep 2019 21:04:52 -0000 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 Received: (qmail 117045 invoked by uid 89); 29 Sep 2019 21:04:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=partially 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; Sun, 29 Sep 2019 21:04:50 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C07CB3083362; Sun, 29 Sep 2019 21:04:49 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-6.rdu2.redhat.com [10.10.112.6]) by smtp.corp.redhat.com (Postfix) with ESMTP id C0F3A5C1B2; Sun, 29 Sep 2019 21:04:48 +0000 (UTC) Subject: Re: [11/32] Remove global call sets: cse.c To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com References: From: Jeff Law Openpgp: preference=signencrypt Message-ID: <66f112a0-6bd8-6c2a-4c85-41c90641964c@redhat.com> Date: Sun, 29 Sep 2019 21:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg01700.txt.bz2 On 9/25/19 9:57 AM, Richard Sandiford wrote: > Richard Sandiford writes: >> Like with the combine.c patch, this one keeps things simple by >> invalidating values in partially-clobbered registers, rather than >> trying to tell whether the value in a partially-clobbered register >> is actually clobbered or not. Again, this is in principle a bug fix, >> but probably never matters in practice. > > Similary to the combine patch, I've updated this to avoid the > short "abi" name and use a temporary HARD_REG_SET instead. > > Richard > > > 2019-09-25 Richard Sandiford > > gcc/ > * cse.c: Include regs.h and function-abi.h. > (invalidate_for_call): Take the call insn as an argument. > Use insn_callee_abi to get the ABI of the call and invalidate > partially clobbered registers as well as fully clobbered ones. > (cse_insn): Update call accordingly. > > Index: gcc/cse.c > =================================================================== > --- gcc/cse.c 2019-09-17 15:27:11.338066929 +0100 > +++ gcc/cse.c 2019-09-25 16:55:31.202641509 +0100 > @@ -42,6 +42,8 @@ Software Foundation; either version 3, o > #include "tree-pass.h" > #include "dbgcnt.h" > #include "rtl-iter.h" > +#include "regs.h" > +#include "function-abi.h" > > /* The basic idea of common subexpression elimination is to go > through the code, keeping a record of expressions that would > @@ -566,7 +568,6 @@ static void remove_invalid_subreg_refs ( > machine_mode); > static void rehash_using_reg (rtx); > static void invalidate_memory (void); > -static void invalidate_for_call (void); > static rtx use_related_value (rtx, struct table_elt *); > > static inline unsigned canon_hash (rtx, machine_mode); > @@ -2091,23 +2092,29 @@ rehash_using_reg (rtx x) > } > > /* Remove from the hash table any expression that is a call-clobbered > - register. Also update their TICK values. */ > + register in INSN. Also update their TICK values. */ > > static void > -invalidate_for_call (void) > +invalidate_for_call (rtx_insn *insn) > { > - unsigned int regno, endregno; > - unsigned int i; > + unsigned int regno; > unsigned hash; > struct table_elt *p, *next; > int in_table = 0; > hard_reg_set_iterator hrsi; > > - /* Go through all the hard registers. For each that is clobbered in > - a CALL_INSN, remove the register from quantity chains and update > + /* Go through all the hard registers. For each that might be clobbered > + in call insn INSN, remove the register from quantity chains and update > reg_tick if defined. Also see if any of these registers is currently > - in the table. */ > - EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi) > + in the table. > + > + ??? We could be more precise for partially-clobbered registers, > + and only invalidate values that actually occupy the clobbered part > + of the registers. It doesn't seem worth the effort though, since > + we shouldn't see this situation much before RA. */ > + HARD_REG_SET callee_clobbers > + = insn_callee_abi (insn).full_and_partial_reg_clobbers (); > + EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, regno, hrsi) > { > delete_reg_equiv (regno); > if (REG_TICK (regno) >= 0) > @@ -2132,15 +2139,11 @@ invalidate_for_call (void) > || REGNO (p->exp) >= FIRST_PSEUDO_REGISTER) > continue; > > - regno = REGNO (p->exp); > - endregno = END_REGNO (p->exp); > - > - for (i = regno; i < endregno; i++) > - if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i)) > - { > - remove_from_table (p, hash); > - break; > - } > + /* This must use the same test as above rather than the > + more accurate clobbers_reg_p. */ > + if (overlaps_hard_reg_set_p (callee_clobbers, GET_MODE (p->exp), > + REGNO (p->exp))) Is it worth putting a forward comment to the earlier test to the later one to help ensure they're kept in sync? OK with or without that comment update.