From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8342 invoked by alias); 2 Jun 2014 19:37:04 -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 8333 invoked by uid 89); 2 Jun 2014 19:37:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-we0-f169.google.com Received: from mail-we0-f169.google.com (HELO mail-we0-f169.google.com) (74.125.82.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 02 Jun 2014 19:37:02 +0000 Received: by mail-we0-f169.google.com with SMTP id u56so5752495wes.28 for ; Mon, 02 Jun 2014 12:36:59 -0700 (PDT) X-Received: by 10.180.218.4 with SMTP id pc4mr12163523wic.21.1401737818968; Mon, 02 Jun 2014 12:36:58 -0700 (PDT) Received: from localhost ([2.26.169.52]) by mx.google.com with ESMTPSA id l9sm35046127wic.21.2014.06.02.12.36.57 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 02 Jun 2014 12:36:58 -0700 (PDT) From: Richard Sandiford To: Matthew Fortune Mail-Followup-To: Matthew Fortune ,Vladimir Makarov , Robert Suchanek , "gcc-patches\@gcc.gnu.org" , Kyrill Tkachov , rdsandiford@googlemail.com Cc: Vladimir Makarov , Robert Suchanek , "gcc-patches\@gcc.gnu.org" , Kyrill Tkachov Subject: RFA: Make LRA temporarily eliminate addresses before testing constraints References: <87d2h51dm6.fsf@talisman.default> <87d2gqfb7t.fsf@talisman.default> <87ob02jodp.fsf@talisman.default> <87fvl6hnw2.fsf@talisman.default> <5357D588.6000202@redhat.com> <87tx967jnq.fsf@talisman.default> <5368EC5F.3010006@arm.com> <87mweuww37.fsf@talisman.default> <6D39441BF12EF246A7ABCE6654B0235352881A@LEMAIL01.le.imgtec.org> <87vbtd1njm.fsf@talisman.default> <871tvqnai3.fsf@talisman.default> Date: Mon, 02 Jun 2014 19:37:00 -0000 In-Reply-To: <871tvqnai3.fsf@talisman.default> (Richard Sandiford's message of "Sun, 18 May 2014 20:37:56 +0100") Message-ID: <8761kjaysm.fsf_-_@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2014-06/txt/msg00146.txt.bz2 Ping. Imagination's copyright assignment has now gone through and so in principle we're ready for the MIPS LRA switch to go in. We need this LRA patch as a prequisite though. Robert: you also had an LRA change, but is it still needed after this one? If so, could you repost it and explain the case it handles? Thanks, Richard Richard Sandiford writes: > Richard Sandiford writes: >> I think a cleaner way of doing it would be to have helper functions >> that switch in and out of the eliminated form, storing the old form >> in fields of a new structure (either separate from address_info, >> or a local inheritance of it). We probably also want to have arrays >> of address_infos, one for each operand, so that we don't analyse the >> same address too many times during the same insn. > > In the end maintaining the array of address_infos seemed like too much > work. It was hard to keep it up-to-date with various other changes > that can be made, including swapping commutative operands, to the point > where it wasn't obvious whether it was really an optimisation or not. > > Here's a patch that does the first. Tested on x86_64-linux-gnu. > This time I also compared the assembly output for gcc.dg, g++.dg > and gcc.c-torture at -O2 on: > > arch64-linux-gnu arm-eabi mipsisa64-sde-elf s390x-linux-gnu > powerpc64-linux-gnu x86_64-linux-gnu > > s390x in particular is very good at exposing problems with this code. > (It caught bugs in the aborted attempt to keep an array of address_infos.) > > OK to install? > > Thanks, > Richard > > > gcc/ > * lra-constraints.c (valid_address_p): Move earlier in file. > (address_eliminator): New structure. > (satisfies_memory_constraint_p): New function. > (satisfies_address_constraint_p): Likewise. > (process_alt_operands, process_address, curr_insn_transform): Use them. > > Index: gcc/lra-constraints.c > =================================================================== > --- gcc/lra-constraints.c 2014-05-17 17:49:19.071639652 +0100 > +++ gcc/lra-constraints.c 2014-05-18 20:36:17.499181467 +0100 > @@ -317,6 +317,118 @@ in_mem_p (int regno) > return get_reg_class (regno) == NO_REGS; > } > > +/* Return 1 if ADDR is a valid memory address for mode MODE in address > + space AS, and check that each pseudo has the proper kind of hard > + reg. */ > +static int > +valid_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, > + rtx addr, addr_space_t as) > +{ > +#ifdef GO_IF_LEGITIMATE_ADDRESS > + lra_assert (ADDR_SPACE_GENERIC_P (as)); > + GO_IF_LEGITIMATE_ADDRESS (mode, addr, win); > + return 0; > + > + win: > + return 1; > +#else > + return targetm.addr_space.legitimate_address_p (mode, addr, 0, as); > +#endif > +} > + > +namespace { > + /* Temporarily eliminates registers in an address (for the lifetime of > + the object). */ > + class address_eliminator { > + public: > + address_eliminator (struct address_info *ad); > + ~address_eliminator (); > + > + private: > + struct address_info *m_ad; > + rtx *m_base_loc; > + rtx m_base_reg; > + rtx *m_index_loc; > + rtx m_index_reg; > + }; > +} > + > +address_eliminator::address_eliminator (struct address_info *ad) > + : m_ad (ad), > + m_base_loc (strip_subreg (ad->base_term)), > + m_base_reg (NULL_RTX), > + m_index_loc (strip_subreg (ad->index_term)), > + m_index_reg (NULL_RTX) > +{ > + if (m_base_loc != NULL) > + { > + m_base_reg = *m_base_loc; > + lra_eliminate_reg_if_possible (m_base_loc); > + if (m_ad->base_term2 != NULL) > + *m_ad->base_term2 = *m_ad->base_term; > + } > + if (m_index_loc != NULL) > + { > + m_index_reg = *m_index_loc; > + lra_eliminate_reg_if_possible (m_index_loc); > + } > +} > + > +address_eliminator::~address_eliminator () > +{ > + if (m_base_loc && *m_base_loc != m_base_reg) > + { > + *m_base_loc = m_base_reg; > + if (m_ad->base_term2 != NULL) > + *m_ad->base_term2 = *m_ad->base_term; > + } > + if (m_index_loc && *m_index_loc != m_index_reg) > + *m_index_loc = m_index_reg; > +} > + > +/* Return true if the eliminated form of AD is a legitimate target address. */ > +static bool > +valid_address_p (struct address_info *ad) > +{ > + address_eliminator eliminator (ad); > + return valid_address_p (ad->mode, *ad->outer, ad->as); > +} > + > +#ifdef EXTRA_CONSTRAINT_STR > +/* Return true if the eliminated form of memory reference OP satisfies > + extra address constraint CONSTRAINT. */ > +static bool > +satisfies_memory_constraint_p (rtx op, const char *constraint) > +{ > + struct address_info ad; > + > + decompose_mem_address (&ad, op); > + address_eliminator eliminator (&ad); > + return EXTRA_CONSTRAINT_STR (op, *constraint, constraint); > +} > + > +/* Return true if the eliminated form of address AD satisfies extra > + address constraint CONSTRAINT. */ > +static bool > +satisfies_address_constraint_p (struct address_info *ad, > + const char *constraint) > +{ > + address_eliminator eliminator (ad); > + return EXTRA_CONSTRAINT_STR (*ad->outer, *constraint, constraint); > +} > + > +/* Return true if the eliminated form of address OP satisfies extra > + address constraint CONSTRAINT. */ > +static bool > +satisfies_address_constraint_p (rtx op, const char *constraint) > +{ > + struct address_info ad; > + > + decompose_lea_address (&ad, &op); > + return satisfies_address_constraint_p (&ad, constraint); > +} > +#endif > + > /* Initiate equivalences for LRA. As we keep original equivalences > before any elimination, we need to make copies otherwise any change > in insns might change the equivalences. */ > @@ -1941,7 +2053,8 @@ process_alt_operands (int only_alternati > #ifdef EXTRA_CONSTRAINT_STR > if (EXTRA_MEMORY_CONSTRAINT (c, p)) > { > - if (EXTRA_CONSTRAINT_STR (op, c, p)) > + if (MEM_P (op) > + && satisfies_memory_constraint_p (op, p)) > win = true; > else if (spilled_pseudo_p (op)) > win = true; > @@ -1960,7 +2073,7 @@ process_alt_operands (int only_alternati > } > if (EXTRA_ADDRESS_CONSTRAINT (c, p)) > { > - if (EXTRA_CONSTRAINT_STR (op, c, p)) > + if (satisfies_address_constraint_p (op, p)) > win = true; > > /* If we didn't already win, we can reload > @@ -2576,60 +2689,6 @@ process_alt_operands (int only_alternati > return ok_p; > } > > -/* Return 1 if ADDR is a valid memory address for mode MODE in address > - space AS, and check that each pseudo has the proper kind of hard > - reg. */ > -static int > -valid_address_p (enum machine_mode mode ATTRIBUTE_UNUSED, > - rtx addr, addr_space_t as) > -{ > -#ifdef GO_IF_LEGITIMATE_ADDRESS > - lra_assert (ADDR_SPACE_GENERIC_P (as)); > - GO_IF_LEGITIMATE_ADDRESS (mode, addr, win); > - return 0; > - > - win: > - return 1; > -#else > - return targetm.addr_space.legitimate_address_p (mode, addr, 0, as); > -#endif > -} > - > -/* Return whether address AD is valid. */ > - > -static bool > -valid_address_p (struct address_info *ad) > -{ > - /* Some ports do not check displacements for eliminable registers, > - so we replace them temporarily with the elimination target. */ > - rtx saved_base_reg = NULL_RTX; > - rtx saved_index_reg = NULL_RTX; > - rtx *base_term = strip_subreg (ad->base_term); > - rtx *index_term = strip_subreg (ad->index_term); > - if (base_term != NULL) > - { > - saved_base_reg = *base_term; > - lra_eliminate_reg_if_possible (base_term); > - if (ad->base_term2 != NULL) > - *ad->base_term2 = *ad->base_term; > - } > - if (index_term != NULL) > - { > - saved_index_reg = *index_term; > - lra_eliminate_reg_if_possible (index_term); > - } > - bool ok_p = valid_address_p (ad->mode, *ad->outer, ad->as); > - if (saved_base_reg != NULL_RTX) > - { > - *base_term = saved_base_reg; > - if (ad->base_term2 != NULL) > - *ad->base_term2 = *ad->base_term; > - } > - if (saved_index_reg != NULL_RTX) > - *index_term = saved_index_reg; > - return ok_p; > -} > - > /* Make reload base reg + disp from address AD. Return the new pseudo. */ > static rtx > base_plus_disp_to_reg (struct address_info *ad) > @@ -2832,7 +2891,7 @@ process_address (int nop, rtx *before, r > EXTRA_CONSTRAINT_STR for the validation. */ > if (constraint[0] != 'p' > && EXTRA_ADDRESS_CONSTRAINT (constraint[0], constraint) > - && EXTRA_CONSTRAINT_STR (op, constraint[0], constraint)) > + && satisfies_address_constraint_p (&ad, constraint)) > return change_p; > #endif > > @@ -3539,7 +3598,7 @@ curr_insn_transform (void) > break; > #ifdef EXTRA_CONSTRAINT_STR > if (EXTRA_MEMORY_CONSTRAINT (c, constraint) > - && EXTRA_CONSTRAINT_STR (tem, c, constraint)) > + && satisfies_memory_constraint_p (tem, constraint)) > break; > #endif > }