This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, loop2_invariant, 1/2] Check only one register class

On 06/11/14 04:05, Zhenqiang Chen wrote:
On 10 June 2014 19:06, Steven Bosscher <> wrote:
On Tue, Jun 10, 2014 at 11:22 AM, Zhenqiang Chen wrote:

For loop2-invariant pass, when flag_ira_loop_pressure is enabled,
function gain_for_invariant checks the pressures of all register
classes. This does not make sense since one invariant might impact
only one register class.

The patch enhances functions get_inv_cost and gain_for_invariant to
check only the register pressure of the invariant if possible.

This patch may work for targets with more-or-less orthogonal reg
classes, but not if there is a lot of overlap between reg classes.

Yes. I need check the overlap between reg classes.

Patch is updated to check all overlap reg classes by reg_classes_intersect_p:
So you need a new ChangeLog, of course :-)

diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
index c76a2a0..6e43b49 100644
--- a/gcc/loop-invariant.c
+++ b/gcc/loop-invariant.c
@@ -1092,16 +1092,22 @@ get_pressure_class_and_nregs (rtx insn, int *nregs)

  /* Calculates cost and number of registers needed for moving invariant INV
-   out of the loop and stores them to *COST and *REGS_NEEDED.  */
+   out of the loop and stores them to *COST and *REGS_NEEDED.  *CL will be
+   the REG_CLASS of INV.  Return
+     0: if INV is invalid.
+     1: if INV and its depends_on have same reg_class
+   > 1: if INV and its depends_on have different reg_classes.  */
Nit/bikeshedding. I tend to prefer < 0, 0, > 0 (or -1, 0, 1) for tri-states. It's not a big deal though.

           check_p = i < ira_pressure_classes_num;
+         if ((dep_ret > 1) || ((dep_ret == 1) && (*cl != dep_cl)))
+           {
+             *cl = ALL_REGS;
+             ret ++;
Whitespace nit -- no space in this statement.  use "ret++;"

You should add a testcase if at all possible. Perhaps two, one which runs on an ARM variant and one for x86_64. The former because that's obviously what Linaro cares about, the latter for wider testing.

Definitely add a ChangeLog entry, fix the whitespace nit & add testcase. OK with those fixes. Your choice on the tri-state values.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]