https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102178

--- Comment #29 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Vladimir Makarov from comment #28)
> Could somebody benchmark the following patch on zen2 470.lbm.
> 
> diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
> index 9cee17479ba..76619aca8eb 100644
> --- a/gcc/lra-constraints.cc
> +++ b/gcc/lra-constraints.cc
> @@ -5084,7 +5089,9 @@ lra_constraints (bool first_p)
>                              (x, lra_get_allocno_class (i)) == NO_REGS))
>                         || contains_symbol_ref_p (x))))
>               ira_reg_equiv[i].defined_p = false;
> -           if (contains_reg_p (x, false, true))
> +           if (contains_reg_p (x, false, true)
> +               || (CONST_DOUBLE_P (x)
> +                   && maybe_ge (GET_MODE_SIZE (GET_MODE (x)), 8)))
>               ira_reg_equiv[i].profitable_p = false;
>             if (get_equiv (reg) != reg)
>               bitmap_ior_into (equiv_insn_bitmap,
> &lra_reg_info[i].insn_bitmap);
> 
> If it improves the performance, I'll commit this patch.
> 
> The expander unconditionally uses memory pool for double constants.  I think
> the analogous treatment could be done for equiv double constants in LRA.
> 
> I know only x86_64 permits 64-bit constants as immediate for moving them
> into general regs.  As double fp operations is not done in general regs in
> the most cases, they should be moved into fp regs and this is costly as Jan
> wrote.  So it has sense to prohibit using equiv double constant values in
> LRA unconditionally.  If in the future we have a target which can move
> double immediate into fp regs we can introduce some target hooks to deal
> with equiv double constant.  But right now I think there is no need for the
> new hook.

Code generation changes quite a bit, with the patch the offending function
is 16 bytes larger.  I see no large immediate moves to GPRs anymore but
there is still a lot of spilling of XMMs to GPRs.  Performance is
unchanged by the patch:

470.lbm         13740        128        107 S   13740        128        107 S
470.lbm         13740        128        107 *   13740        128        107 S
470.lbm         13740        128        107 S   13740        128        107 *

I've used

diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 9cee17479ba..a0ec608c056 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -5084,7 +5084,9 @@ lra_constraints (bool first_p)
                             (x, lra_get_allocno_class (i)) == NO_REGS))
                        || contains_symbol_ref_p (x))))
              ira_reg_equiv[i].defined_p = false;
-           if (contains_reg_p (x, false, true))
+           if (contains_reg_p (x, false, true)
+               || (CONST_DOUBLE_P (x)
+                   && maybe_ge (GET_MODE_SIZE (GET_MODE (x)),
UNITS_PER_WORD)))
              ira_reg_equiv[i].profitable_p = false;
            if (get_equiv (reg) != reg)
              bitmap_ior_into (equiv_insn_bitmap,
&lra_reg_info[i].insn_bitmap);

note UNITS_PER_WORD vs. literal 8.

Without knowing much of the code I wonder if we can check whether the move
will be to a reg in GENERAL_REGS?  That is, do we know whether there are
(besides some special constants like zero), immediate moves to the
destination register class?

That said, given the result on LBM I'd not change this at this point.

Honza wanted to look at the move pattern to try to mitigate the
GPR spilling of XMMs.

I do think that we need to take costs into account at some point and get
rid of the reload style hand-waving with !?* in the move patterns.

Reply via email to