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: RFA: patch fixing PR37948


Jeff Law wrote:
Vladimir Makarov wrote:
The following patch solves PR37948. The analysis of the problem can be found

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37948

The patch also switches on extended coalescing in IRA which can be used also for solving the problem.

I found some pitfalls in coalescing code (usage of allocnos itself instead of head of coalescing allocnos list) and fixed that.

The patch was successfully bootstrapped and tested on x86/x86_64.

2008-11-07  Vladimir Makarov  <vmakarov@redhat.com>
         PR rtl-optimizations/37948
   * ira-int.h (struct ira_allocno_copy): New member constraint_p.
   (ira_create_copy, ira_add_allocno_copy): New parameter.

   * ira-conflicts.c (process_regs_for_copy): New parameter.  Pass it
   to ira_add_allocno_copy.
   (process_reg_shuffles, add_insn_allocno_copies): Pass a new
   parameter to process_regs_for_copy.
   (propagate_copies): Pass a new parameter to ira_add_allocno_copy.
   Fix typo in passing second allocno to ira_add_allocno_copy.

   * ira-color.c (update_conflict_hard_regno_costs): Use head of
   coalesced allocnos list.
   (assign_hard_reg): Ditto.  Check that assigned allocnos are not in
   the graph.
   (add_ira_allocno_to_bucket): Rename to add_allocno_to_bucket.
   (add_ira_allocno_to_ordered_bucket): Rename to
   add_allocno_to_ordered_bucket.
   (push_ira_allocno_to_stack): Rename to push_allocno_to_stack.  Use
   head of coalesced allocnos list.
   (push_allocnos_to_stack): Remove calculation of ALLOCNO_TEMP.
   Check that it is aready calculated.
   (push_ira_allocno_to_spill): Rename to push_ira_allocno_to_spill.
   (setup_allocno_left_conflicts_num): Use head of coalesced allocnos
   list.
   (coalesce_allocnos): Do extended coalescing too.

   * ira-emit.c (add_range_and_copies_from_move_list): Pass a new
   parameter to ira_add_allocno_copy.

   * ira-build.c (ira_create_copy, ira_add_allocno_copy): Add a new
   parameter.
   (print_copy): Print copy origination too.

* ira-costs.c (scan_one_insn): Use alloc_pref for load from
equivalent memory.
So presumably after these changes your opinion is that extended coalescing no longer generates worse code?

No, my benchmarking shows that usage -fira-coalesce (extended or not) gives a bit worse code on SPEC2000 for x86/x86_64. Therefore the option is not default. But usage of extended coalescing fixes the PR. So I think for some code -fira-coalesce (with implemented extended coalescing) could be helpful for some programs.

I think we should work more on coalescing. I tried Appel/George iterative coalescing recently. In some cases it generates a better SPEC2000 scores (like x86) but in other cases it generates a bit worse code (like x86_64 or ppc SPEC2000). But at least iterative coalescing works much better than the current coalescing (I implemented kind of optimistic coalescing. I remember one cgo2007 article which was awarded as the best cg2007 article showing that optimistic coalescing is the best).

Do we need both the extended coalescing and cost change (scan_one_insn) to address the performance regression?

Scan_one_insn is enough. In this patch I also fixed some coalescing code. Probably I should have submitted them as separate patches.

The patch is OK -- I'm just trying to make sure the interaction between the two hunks is understood and recorded if there are future issues.
Jeff, thanks for the instant response.


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