* patch to fix PR59511
@ 2014-01-15 17:34 Vladimir Makarov
2014-01-15 22:44 ` H.J. Lu
0 siblings, 1 reply; 2+ messages in thread
From: Vladimir Makarov @ 2014-01-15 17:34 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]
The following patch fixes:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59511
Register move cost calculations were too conservative for some cases
and it resulted into overflows. The patch fixes also some wrong
frequencies usage I found.
The patch has very small affect on SPEC2000. Only 7 programs out of
26 has a different code for -mtune=corei7. For -mtune=generic, it is
only 3 programs. And there is no visible changes in performance for
given programs. So I'd say this PR is not worth P1 status.
The patch was successfully bootstrapped and tested on x86/x86-64.
Committed as rev. 206636.
2014-01-15 Vladimir Makarov <vmakarov@redhat.com>
PR rtl-optimization/59511
* ira.c (ira_init_register_move_cost): Use memory costs for some
cases of register move cost calculations.
* lra-constraints.c (lra_constraints): Use REG_FREQ_FROM_BB
instead of BB frequency.
* lra-coalesce.c (move_freq_compare_func, lra_coalesce): Ditto.
* lra-assigns.c (find_hard_regno_for): Ditto.
[-- Attachment #2: pr59511.patch --]
[-- Type: text/plain, Size: 7727 bytes --]
Index: ira.c
===================================================================
--- ira.c (revision 206609)
+++ ira.c (working copy)
@@ -1574,21 +1574,30 @@ ira_init_register_move_cost (enum machin
&& ira_may_move_out_cost[mode] == NULL);
ira_assert (have_regs_of_mode[mode]);
for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
- if (contains_reg_of_mode[cl1][mode])
- for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
- {
- int cost;
- if (!contains_reg_of_mode[cl2][mode])
- cost = 65535;
- else
- {
- cost = register_move_cost (mode, (enum reg_class) cl1,
- (enum reg_class) cl2);
- ira_assert (cost < 65535);
- }
- all_match &= (last_move_cost[cl1][cl2] == cost);
- last_move_cost[cl1][cl2] = cost;
- }
+ for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
+ {
+ int cost;
+ if (!contains_reg_of_mode[cl1][mode]
+ || !contains_reg_of_mode[cl2][mode])
+ {
+ if ((ira_reg_class_max_nregs[cl1][mode]
+ > ira_class_hard_regs_num[cl1])
+ || (ira_reg_class_max_nregs[cl2][mode]
+ > ira_class_hard_regs_num[cl2]))
+ cost = 65535;
+ else
+ cost = (ira_memory_move_cost[mode][cl1][0]
+ + ira_memory_move_cost[mode][cl2][1]);
+ }
+ else
+ {
+ cost = register_move_cost (mode, (enum reg_class) cl1,
+ (enum reg_class) cl2);
+ ira_assert (cost < 65535);
+ }
+ all_match &= (last_move_cost[cl1][cl2] == cost);
+ last_move_cost[cl1][cl2] = cost;
+ }
if (all_match && last_mode_for_init_move_cost != -1)
{
ira_register_move_cost[mode]
@@ -1604,58 +1613,51 @@ ira_init_register_move_cost (enum machin
ira_may_move_in_cost[mode] = XNEWVEC (move_table, N_REG_CLASSES);
ira_may_move_out_cost[mode] = XNEWVEC (move_table, N_REG_CLASSES);
for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
- if (contains_reg_of_mode[cl1][mode])
- for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
- {
- int cost;
- enum reg_class *p1, *p2;
-
- if (last_move_cost[cl1][cl2] == 65535)
- {
- ira_register_move_cost[mode][cl1][cl2] = 65535;
- ira_may_move_in_cost[mode][cl1][cl2] = 65535;
- ira_may_move_out_cost[mode][cl1][cl2] = 65535;
- }
- else
- {
- cost = last_move_cost[cl1][cl2];
-
- for (p2 = ®_class_subclasses[cl2][0];
- *p2 != LIM_REG_CLASSES; p2++)
- if (ira_class_hard_regs_num[*p2] > 0
- && (ira_reg_class_max_nregs[*p2][mode]
- <= ira_class_hard_regs_num[*p2]))
- cost = MAX (cost, ira_register_move_cost[mode][cl1][*p2]);
-
- for (p1 = ®_class_subclasses[cl1][0];
- *p1 != LIM_REG_CLASSES; p1++)
- if (ira_class_hard_regs_num[*p1] > 0
- && (ira_reg_class_max_nregs[*p1][mode]
- <= ira_class_hard_regs_num[*p1]))
- cost = MAX (cost, ira_register_move_cost[mode][*p1][cl2]);
-
- ira_assert (cost <= 65535);
- ira_register_move_cost[mode][cl1][cl2] = cost;
-
- if (ira_class_subset_p[cl1][cl2])
- ira_may_move_in_cost[mode][cl1][cl2] = 0;
- else
- ira_may_move_in_cost[mode][cl1][cl2] = cost;
-
- if (ira_class_subset_p[cl2][cl1])
- ira_may_move_out_cost[mode][cl1][cl2] = 0;
- else
- ira_may_move_out_cost[mode][cl1][cl2] = cost;
- }
- }
- else
- for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
- {
- ira_register_move_cost[mode][cl1][cl2] = 65535;
- ira_may_move_in_cost[mode][cl1][cl2] = 65535;
- ira_may_move_out_cost[mode][cl1][cl2] = 65535;
- }
+ for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
+ {
+ int cost;
+ enum reg_class *p1, *p2;
+
+ if (last_move_cost[cl1][cl2] == 65535)
+ {
+ ira_register_move_cost[mode][cl1][cl2] = 65535;
+ ira_may_move_in_cost[mode][cl1][cl2] = 65535;
+ ira_may_move_out_cost[mode][cl1][cl2] = 65535;
+ }
+ else
+ {
+ cost = last_move_cost[cl1][cl2];
+
+ for (p2 = ®_class_subclasses[cl2][0];
+ *p2 != LIM_REG_CLASSES; p2++)
+ if (ira_class_hard_regs_num[*p2] > 0
+ && (ira_reg_class_max_nregs[*p2][mode]
+ <= ira_class_hard_regs_num[*p2]))
+ cost = MAX (cost, ira_register_move_cost[mode][cl1][*p2]);
+
+ for (p1 = ®_class_subclasses[cl1][0];
+ *p1 != LIM_REG_CLASSES; p1++)
+ if (ira_class_hard_regs_num[*p1] > 0
+ && (ira_reg_class_max_nregs[*p1][mode]
+ <= ira_class_hard_regs_num[*p1]))
+ cost = MAX (cost, ira_register_move_cost[mode][*p1][cl2]);
+
+ ira_assert (cost <= 65535);
+ ira_register_move_cost[mode][cl1][cl2] = cost;
+
+ if (ira_class_subset_p[cl1][cl2])
+ ira_may_move_in_cost[mode][cl1][cl2] = 0;
+ else
+ ira_may_move_in_cost[mode][cl1][cl2] = cost;
+
+ if (ira_class_subset_p[cl2][cl1])
+ ira_may_move_out_cost[mode][cl1][cl2] = 0;
+ else
+ ira_may_move_out_cost[mode][cl1][cl2] = cost;
+ }
+ }
}
+
\f
/* This is called once during compiler work. It sets up
Index: lra-assigns.c
===================================================================
--- lra-assigns.c (revision 206609)
+++ lra-assigns.c (working copy)
@@ -612,7 +612,9 @@ find_hard_regno_for (int regno, int *cos
&& ! df_regs_ever_live_p (hard_regno + j))
/* It needs save restore. */
hard_regno_costs[hard_regno]
- += 2 * ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->frequency + 1;
+ += (2
+ * REG_FREQ_FROM_BB (ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb)
+ + 1);
priority = targetm.register_priority (hard_regno);
if (best_hard_regno < 0 || hard_regno_costs[hard_regno] < best_cost
|| (hard_regno_costs[hard_regno] == best_cost
Index: lra-coalesce.c
===================================================================
--- lra-coalesce.c (revision 206609)
+++ lra-coalesce.c (working copy)
@@ -79,8 +79,8 @@ move_freq_compare_func (const void *v1p,
rtx mv2 = *(const rtx *) v2p;
int pri1, pri2;
- pri1 = BLOCK_FOR_INSN (mv1)->frequency;
- pri2 = BLOCK_FOR_INSN (mv2)->frequency;
+ pri1 = REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv1));
+ pri2 = REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv2));
if (pri2 - pri1)
return pri2 - pri1;
@@ -277,7 +277,7 @@ lra_coalesce (void)
fprintf
(lra_dump_file, " Coalescing move %i:r%d-r%d (freq=%d)\n",
INSN_UID (mv), sregno, dregno,
- BLOCK_FOR_INSN (mv)->frequency);
+ REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv)));
/* We updated involved_insns_bitmap when doing the merge. */
}
else if (!(lra_intersected_live_ranges_p
@@ -291,7 +291,7 @@ lra_coalesce (void)
" Coalescing move %i:r%d(%d)-r%d(%d) (freq=%d)\n",
INSN_UID (mv), sregno, ORIGINAL_REGNO (SET_SRC (set)),
dregno, ORIGINAL_REGNO (SET_DEST (set)),
- BLOCK_FOR_INSN (mv)->frequency);
+ REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv)));
bitmap_ior_into (&involved_insns_bitmap,
&lra_reg_info[sregno].insn_bitmap);
bitmap_ior_into (&involved_insns_bitmap,
@@ -316,7 +316,8 @@ lra_coalesce (void)
/* Coalesced move. */
if (lra_dump_file != NULL)
fprintf (lra_dump_file, " Removing move %i (freq=%d)\n",
- INSN_UID (insn), BLOCK_FOR_INSN (insn)->frequency);
+ INSN_UID (insn),
+ REG_FREQ_FROM_BB (BLOCK_FOR_INSN (insn)));
lra_set_insn_deleted (insn);
}
}
Index: lra-constraints.c
===================================================================
--- lra-constraints.c (revision 206609)
+++ lra-constraints.c (working copy)
@@ -4077,7 +4077,7 @@ lra_constraints (bool first_p)
fprintf (lra_dump_file,
" Removing equiv init insn %i (freq=%d)\n",
INSN_UID (curr_insn),
- BLOCK_FOR_INSN (curr_insn)->frequency);
+ REG_FREQ_FROM_BB (BLOCK_FOR_INSN (curr_insn)));
dump_insn_slim (lra_dump_file, curr_insn);
}
if (contains_reg_p (x, true, false))
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: patch to fix PR59511
2014-01-15 17:34 patch to fix PR59511 Vladimir Makarov
@ 2014-01-15 22:44 ` H.J. Lu
0 siblings, 0 replies; 2+ messages in thread
From: H.J. Lu @ 2014-01-15 22:44 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: GCC Patches
On Wed, Jan 15, 2014 at 9:34 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> The following patch fixes:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59511
>
> Register move cost calculations were too conservative for some cases
> and it resulted into overflows. The patch fixes also some wrong
> frequencies usage I found.
>
> The patch has very small affect on SPEC2000. Only 7 programs out of
> 26 has a different code for -mtune=corei7. For -mtune=generic, it is
> only 3 programs. And there is no visible changes in performance for
> given programs. So I'd say this PR is not worth P1 status.
>
> The patch was successfully bootstrapped and tested on x86/x86-64.
>
> Committed as rev. 206636.
>
>
> 2014-01-15 Vladimir Makarov <vmakarov@redhat.com>
>
> PR rtl-optimization/59511
> * ira.c (ira_init_register_move_cost): Use memory costs for some
> cases of register move cost calculations.
> * lra-constraints.c (lra_constraints): Use REG_FREQ_FROM_BB
> instead of BB frequency.
> * lra-coalesce.c (move_freq_compare_func, lra_coalesce): Ditto.
> * lra-assigns.c (find_hard_regno_for): Ditto.
This caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59835
--
H.J.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-01-15 22:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-15 17:34 patch to fix PR59511 Vladimir Makarov
2014-01-15 22:44 ` H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).