This is the mail archive of the gcc-patches@gcc.gnu.org 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: IRA improvements 3/4


Jeff Law wrote:
On 07/07/10 07:31, Vladimir Makarov wrote:
 The following patch speeds up IRA by decreasing number of processed
register classes for allocno.  To decrease the number, classes
specific for given allocno is used.  For example, if allocno of DIMode
and FLOAT_REGS can not hold such value, we should not try FLOAT_REGS
or if we decided that only floating point registers are profitable for
given allocno on the 1st pass, we most probably should not try general
registers on the 2nd pass.

Is the patch ok to commit the patch to the trunk?

2010-07-07 Vladimir Makarov <vmakarov@redhat.com>

   * ira-costs.c (cost_classes, cost_classes_num): Remove.
   (struct cost_classes, cost_classes_t, const_cost_classes_t,
   regno_cost_classes, cost_classes_hash, cost_classes_eq,
   cost_classes_del, cost_classes_htab, cost_classes_mode_cache,
   initiate_regno_cost_classes, setup_cost_classes,
   setup_regno_cost_classes_by_aclass,
   setup_regno_cost_classes_by_mode, finish_regno_cost_classes): New.
   (record_reg_classes): Use regno_cost_classes instead of
   cost_classes.
   (record_address_regs, scan_one_insn): Ditto.
   (print_allocno_costs, print_pseudo_costs): Ditto.
   (find_costs_and_classes): Set up cost classes for each registers.
   Use also their mode for this.  Use regno_cost_classes instead of
   cost_classes.
   (setup_allocno_class_and_costs): Use regno_cost_classes instead of
   cost_classes.
   (free_ira_costs, ira_init_costs): Don't use cost_classes.
   (ira_costs, ira_set_pseudo_classes): Call
   initiate_regno_cost_classes and finish_regno_cost_classes.


nit: s/memebrs/members/

-          for (k = 0; k < cost_classes_num; k++)
+          for (k = cost_classes_ptr->num - 1; k >= 0; k--)
Presumably you made this change to improve the performance of the loop (and others like it) and presumably the loop body is too complex for GCC's loop reversal to apply automatically and LICM couldn't remove the memory reference cost_classes_ptr->num from each iteration of the loop?


Yes, usually I expect that this kind of optimizations (moving invariant and removing living pseudo by loop reversal by this case) are done by gcc. But when I check the assembler code, it is sometimes not what I expected. As I remember that was the case for this critical loop when I compiled by gcc4.3.


Nit:
- if (REG_P (ops[i]) && REGNO (ops[i]) >= FIRST_PSEUDO_REGISTER)
+ if (REG_P (ops[i]) && (regno = REGNO (ops[i])) >= FIRST_PSEUDO_REGISTER)
You seem to introduce this style of code in several places. I've never been a big fan as I think it makes the code harder to read and more prone to silly operator precedence errors (particularly when someone else changes things later). If we can cleanly avoid such code, let's do so. If this style is the cleanest way to express the conditional, then I'm not going to get terribly bent out of shape about it.

In this case, it can be removed for clean code. I'll do it. Although I'd say it is quite a frequent practice in gcc.
Based on your description, I'm assuming this patch is primarily designed to help compile-time after removal of cover classes, right? From my reading, it may still be applicable to IRA in the mainline today, though perhaps without providing real compile-time benefts. If so, I wouldn't object to this going into the mainline after separate test/bootstrap cycle if you wanted to reduce the divergence of your ira-improv branch.

Yes, right. In general, it would be easier for me to merge the branch into the mainline. I spent some time to divide the branch changes into several patches mainly for easier review. The order of applying patches are cover.patch, compress.patch, costs.patch and finally speedup.patch. Only compress.patch is independent.

My plan is now to implement your and other proposals on the branch, wait for Bernd's patches on the trunk, merge the branch with the trunk, resolve the conflicts, and submitting the patches again.


Thanks for the review. I really appreciate that you looked at these big patches so quickly.



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