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 <rdsandif...@googlemail.com> writes:
> Richard Sandiford <rdsandif...@googlemail.com> 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
> }
