| Message ID | CACgzC7Bxg4uz=CANZra7FoiGzKZKxpKp8Jz0oGPPrE+g+XiR5w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 06/11/14 04:05, Zhenqiang Chen wrote: > On 10 June 2014 19:06, Steven Bosscher <stevenb.gcc@gmail.com> wrote: >> On Tue, Jun 10, 2014 at 11:22 AM, Zhenqiang Chen wrote: >>> Hi, >>> >>> 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: Just so I'm sure I know what you're trying to do. You want to map the pseudo back to its likely class(es) then look at how those classes (and only those classes) would be impacted from a register pressure standpoint if the pseudo was hoisted as an invariant? This is primarily achieved by returning the class of the invariant, then filtering out any non-intersecting classes in gain_for_invariant, right? jeff
On 18 June 2014 05:49, Jeff Law <law@redhat.com> wrote: > On 06/11/14 04:05, Zhenqiang Chen wrote: >> >> On 10 June 2014 19:06, Steven Bosscher <stevenb.gcc@gmail.com> wrote: >>> >>> On Tue, Jun 10, 2014 at 11:22 AM, Zhenqiang Chen wrote: >>>> >>>> Hi, >>>> >>>> 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: > > Just so I'm sure I know what you're trying to do. > > You want to map the pseudo back to its likely class(es) then look at how > those classes (and only those classes) would be impacted from a register > pressure standpoint if the pseudo was hoisted as an invariant? Yes. > This is primarily achieved by returning the class of the invariant, then > filtering out any non-intersecting classes in gain_for_invariant, right? Yes. This is what I want to do since I found some invariant which register class is NO_REGS (memory write) or SSE_REGS is blocked by GENERAL_REGS' register pressure. Thanks! -Zhenqiang
On 06/11/14 04:05, Zhenqiang Chen wrote: > On 10 June 2014 19:06, Steven Bosscher <stevenb.gcc@gmail.com> wrote: >> On Tue, Jun 10, 2014 at 11:22 AM, Zhenqiang Chen wrote: >>> Hi, >>> >>> 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. jeff
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. */ -static void -get_inv_cost (struct invariant *inv, int *comp_cost, unsigned *regs_needed) +static int +get_inv_cost (struct invariant *inv, int *comp_cost, unsigned *regs_needed, + enum reg_class *cl) { int i, acomp_cost; unsigned aregs_needed[N_REG_CLASSES]; unsigned depno; struct invariant *dep; bitmap_iterator bi; + int ret = 2; /* Find the representative of the class of the equivalent invariants. */ inv = invariants[inv->eqto]; @@ -1117,7 +1123,7 @@ get_inv_cost (struct invariant *inv, int *comp_cost, unsigned *regs_needed) if (inv->move || inv->stamp == actual_stamp) - return; + return 0; inv->stamp = actual_stamp;