* patch to fix PR88282
@ 2018-12-04 15:11 Vladimir Makarov
2018-12-04 16:13 ` Richard Sandiford
0 siblings, 1 reply; 2+ messages in thread
From: Vladimir Makarov @ 2018-12-04 15:11 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 206 bytes --]
 The following patch fixes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88282
 The patch was successfully bootstrapped and tested on
x86/x86-64/ppc64/aarch64.
 Committed as rev. 266784.
[-- Attachment #2: pr88282.patch --]
[-- Type: text/x-patch, Size: 3438 bytes --]
Index: ChangeLog
===================================================================
--- ChangeLog (revision 266783)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2018-12-04 Vladimir Makarov <vmakarov@redhat.com>
+
+ PR target/88282
+ * ira-costs.c (exec): Try bigger class to use smaller register
+ move cost.
+
2018-12-04 Michael Ploujnikov <michael.ploujnikov@oracle.com>
PR ipa/88297
Index: ira-costs.c
===================================================================
--- ira-costs.c (revision 266678)
+++ ira-costs.c (working copy)
@@ -1314,28 +1314,50 @@ record_operand_costs (rtx_insn *insn, en
machine_mode mode = GET_MODE (SET_SRC (set));
cost_classes_t cost_classes_ptr = regno_cost_classes[regno];
enum reg_class *cost_classes = cost_classes_ptr->classes;
- reg_class_t rclass, hard_reg_class, pref_class;
+ reg_class_t rclass, hard_reg_class, pref_class, bigger_hard_reg_class;
int cost, k;
+ move_table *move_costs;
bool dead_p = find_regno_note (insn, REG_DEAD, REGNO (src));
ira_init_register_move_cost_if_necessary (mode);
+ move_costs = ira_register_move_cost[mode];
hard_reg_class = REGNO_REG_CLASS (other_regno);
+ bigger_hard_reg_class = ira_pressure_class_translate[hard_reg_class];
+ if (bigger_hard_reg_class == NO_REGS
+ && (other_regno == STACK_POINTER_REGNUM
+#ifdef STATIC_CHAIN_REGNUM
+ || other_regno == STATIC_CHAIN_REGNUM
+#endif
+ || other_regno == FRAME_POINTER_REGNUM
+ || other_regno == HARD_FRAME_POINTER_REGNUM))
+ bigger_hard_reg_class = GENERAL_REGS;
/* Target code may return any cost for mode which does not
fit the the hard reg class (e.g. DImode for AREG on
i386). Check this and use a bigger class to get the
right cost. */
if (! ira_hard_reg_in_set_p (other_regno, mode,
reg_class_contents[hard_reg_class]))
- hard_reg_class = ira_pressure_class_translate[hard_reg_class];
+ hard_reg_class = bigger_hard_reg_class;
i = regno == (int) REGNO (src) ? 1 : 0;
for (k = cost_classes_ptr->num - 1; k >= 0; k--)
{
rclass = cost_classes[k];
- cost = ((i == 0
- ? ira_register_move_cost[mode][hard_reg_class][rclass]
- : ira_register_move_cost[mode][rclass][hard_reg_class])
- * frequency);
- op_costs[i]->cost[k] = cost;
+ cost = (i == 0
+ ? move_costs[hard_reg_class][rclass]
+ : move_costs[rclass][hard_reg_class]);
+ /* Target code might define wrong big costs for smaller
+ reg classes or reg classes containing only fixed hard
+ regs. Try a bigger class. */
+ if (bigger_hard_reg_class != hard_reg_class)
+ {
+ int cost2 = (i == 0
+ ? move_costs[bigger_hard_reg_class][rclass]
+ : move_costs[rclass][bigger_hard_reg_class]);
+ if (cost2 < cost)
+ cost = cost2;
+ }
+
+ op_costs[i]->cost[k] = cost * frequency;
/* If we have assigned a class to this allocno in our
first pass, add a cost to this alternative
corresponding to what we would add if this allocno
@@ -1350,7 +1372,7 @@ record_operand_costs (rtx_insn *insn, en
else if (ira_reg_class_intersect[pref_class][rclass]
== NO_REGS)
op_costs[i]->cost[k]
- += (ira_register_move_cost[mode][pref_class][rclass]
+ += (move_costs[pref_class][rclass]
* frequency);
}
/* If this insn is a single set copying operand 1 to
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: patch to fix PR88282
2018-12-04 15:11 patch to fix PR88282 Vladimir Makarov
@ 2018-12-04 16:13 ` Richard Sandiford
0 siblings, 0 replies; 2+ messages in thread
From: Richard Sandiford @ 2018-12-04 16:13 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: gcc-patches
Vladimir Makarov <vmakarov@redhat.com> writes:
> The following patch fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88282
>
> The patch was successfully bootstrapped and tested on
> x86/x86-64/ppc64/aarch64.
>
> Committed as rev. 266784.
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 266783)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,9 @@
> +2018-12-04 Vladimir Makarov <vmakarov@redhat.com>
> +
> + PR target/88282
> + * ira-costs.c (exec): Try bigger class to use smaller register
> + move cost.
> +
> 2018-12-04 Michael Ploujnikov <michael.ploujnikov@oracle.com>
>
> PR ipa/88297
> Index: ira-costs.c
> ===================================================================
> --- ira-costs.c (revision 266678)
> +++ ira-costs.c (working copy)
> @@ -1314,28 +1314,50 @@ record_operand_costs (rtx_insn *insn, en
> machine_mode mode = GET_MODE (SET_SRC (set));
> cost_classes_t cost_classes_ptr = regno_cost_classes[regno];
> enum reg_class *cost_classes = cost_classes_ptr->classes;
> - reg_class_t rclass, hard_reg_class, pref_class;
> + reg_class_t rclass, hard_reg_class, pref_class, bigger_hard_reg_class;
> int cost, k;
> + move_table *move_costs;
> bool dead_p = find_regno_note (insn, REG_DEAD, REGNO (src));
>
> ira_init_register_move_cost_if_necessary (mode);
> + move_costs = ira_register_move_cost[mode];
> hard_reg_class = REGNO_REG_CLASS (other_regno);
> + bigger_hard_reg_class = ira_pressure_class_translate[hard_reg_class];
> + if (bigger_hard_reg_class == NO_REGS
> + && (other_regno == STACK_POINTER_REGNUM
> +#ifdef STATIC_CHAIN_REGNUM
> + || other_regno == STATIC_CHAIN_REGNUM
> +#endif
> + || other_regno == FRAME_POINTER_REGNUM
> + || other_regno == HARD_FRAME_POINTER_REGNUM))
> + bigger_hard_reg_class = GENERAL_REGS;
> /* Target code may return any cost for mode which does not
> fit the the hard reg class (e.g. DImode for AREG on
> i386). Check this and use a bigger class to get the
> right cost. */
> if (! ira_hard_reg_in_set_p (other_regno, mode,
> reg_class_contents[hard_reg_class]))
> - hard_reg_class = ira_pressure_class_translate[hard_reg_class];
> + hard_reg_class = bigger_hard_reg_class;
> i = regno == (int) REGNO (src) ? 1 : 0;
> for (k = cost_classes_ptr->num - 1; k >= 0; k--)
> {
> rclass = cost_classes[k];
> - cost = ((i == 0
> - ? ira_register_move_cost[mode][hard_reg_class][rclass]
> - : ira_register_move_cost[mode][rclass][hard_reg_class])
> - * frequency);
> - op_costs[i]->cost[k] = cost;
> + cost = (i == 0
> + ? move_costs[hard_reg_class][rclass]
> + : move_costs[rclass][hard_reg_class]);
> + /* Target code might define wrong big costs for smaller
> + reg classes or reg classes containing only fixed hard
> + regs. Try a bigger class. */
> + if (bigger_hard_reg_class != hard_reg_class)
> + {
> + int cost2 = (i == 0
> + ? move_costs[bigger_hard_reg_class][rclass]
> + : move_costs[rclass][bigger_hard_reg_class]);
> + if (cost2 < cost)
> + cost = cost2;
> + }
> +
> + op_costs[i]->cost[k] = cost * frequency;
> /* If we have assigned a class to this allocno in our
> first pass, add a cost to this alternative
> corresponding to what we would add if this allocno
This seems like a hack to me. I don't see any reason why
FRAME_POINTER_REGNUM etc. have to be in GENERAL_REGS, so picking
that seems arbitrary. And is there a reason not to include
ARG_POINTER_REGNUM or PIC_OFFSET_TABLE_REGNUM?
It sounds from the comments in the PR that the backend isn't defining
its costs correctly. If so, what specifically does it get wrong?
I think it would be better to fix the backends than accumulate more
workarounds like this.
Thanks,
Richard
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-12-04 16:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-04 15:11 patch to fix PR88282 Vladimir Makarov
2018-12-04 16:13 ` Richard Sandiford
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).