Re: [PATCH][RFA][target/79404] Fix uninitialized reference to ira_register_move_cost[mode]
On Wed, 15 Feb 2017, Jeff Law wrote: >> .../gerald/gcc-HEAD/libquadmath/math/remainderq.c:67:1: internal >> compiler error: in ira_init_register_move_cost, at ira.c:1580 > Already testing a fix. Thanks, Jeff. Just to confirm that things are back to bootstrap land. Gerald
Re: [PATCH][RFA][target/79404] Fix uninitialized reference to ira_register_move_cost[mode]
On 02/15/2017 12:42 PM, Gerald Pfeifer wrote: Hi Jeff, On Mon, 13 Feb 2017, Jeff Law wrote: I've verified this allows libgcc to build on the H8 target and bootstrapped/regression tested the change on x86_64-unknown-linux-gnu as well. I need to reproduce this, but my latest daily bootstrap on i386-unknown-freebsd10.3 failed with... .../gerald/gcc-HEAD/libquadmath/math/remainderq.c: In function 'remainderq': .../gerald/gcc-HEAD/libquadmath/math/remainderq.c:67:1: internal compiler error: in ira_init_register_move_cost, at ira.c:1580 ...and your message was the only one on gcc-patches the last couple of days that contains the string "ira_init_register_move_cost". Not sure yet that it's your patch, but this looks a little bit like a smoking gun... :-) Already testing a fix. jeff
Re: [PATCH][RFA][target/79404] Fix uninitialized reference to ira_register_move_cost[mode]
Hi Jeff, On Mon, 13 Feb 2017, Jeff Law wrote: I've verified this allows libgcc to build on the H8 target and bootstrapped/regression tested the change on x86_64-unknown-linux-gnu as well. I need to reproduce this, but my latest daily bootstrap on i386-unknown-freebsd10.3 failed with... .../gerald/gcc-HEAD/libquadmath/math/remainderq.c: In function 'remainderq': .../gerald/gcc-HEAD/libquadmath/math/remainderq.c:67:1: internal compiler error: in ira_init_register_move_cost, at ira.c:1580 ...and your message was the only one on gcc-patches the last couple of days that contains the string "ira_init_register_move_cost". Not sure yet that it's your patch, but this looks a little bit like a smoking gun... :-) Gerald
Re: [PATCH][RFA][target/79404] Fix uninitialized reference to ira_register_move_cost[mode]
On 02/14/2017 01:30 AM, Jeff Law wrote:
So imagine we have two allocnos related by a copy chain (two operand
architecture).
(gdb) p *cp->first
$11 = {num = 9, regno = 33, mode = DImode, wmode = DImode, aclass =
GENERAL_REGS, dont_reassign_p = 0,
bad_spill_p = 0, assigned_p = 1, conflict_vec_p = 0, hard_regno =
-1, next_regno_allocno = 0x0,
loop_tree_node = 0x1e0b190, nrefs = 13, freq = 8069, class_cost =
1380, updated_class_cost = 1380,
memory_cost = 29656, updated_memory_cost = 29656,
excess_pressure_points_num = 17, allocno_prefs = 0x0,
allocno_copies = 0x1e4b400, cap = 0x0, cap_member = 0x0, num_objects
= 1, objects = {0x1e8b6a0, 0x0},
call_freq = 0, calls_crossed_num = 0, cheap_calls_crossed_num = 0,
crossed_calls_clobbered_regs = 0,
hard_reg_costs = 0x1da9510, updated_hard_reg_costs = 0x0,
conflict_hard_reg_costs = 0x0,
updated_conflict_hard_reg_costs = 0x0, add_data = 0x1e04378}
(gdb) p *cp->second
$12 = {num = 12, regno = 39, mode = SImode, wmode = SImode, aclass =
GENERAL_REGS, dont_reassign_p = 0,
bad_spill_p = 1, assigned_p = 1, conflict_vec_p = 0, hard_regno = 2,
next_regno_allocno = 0x0,
loop_tree_node = 0x1e0b190, nrefs = 2, freq = 388, class_cost = 0,
updated_class_cost = 0, memory_cost = 1552,
updated_memory_cost = 1552, excess_pressure_points_num = 0,
allocno_prefs = 0x0, allocno_copies = 0x1e4b400,
cap = 0x0, cap_member = 0x0, num_objects = 2, objects = {0x1e8b7e0,
0x1e8b830}, call_freq = 0,
calls_crossed_num = 0, cheap_calls_crossed_num = 0,
crossed_calls_clobbered_regs = 0,
hard_reg_costs = 0x1da9550, updated_hard_reg_costs = 0x0,
conflict_hard_reg_costs = 0x0,
updated_conflict_hard_reg_costs = 0x0, add_data = 0x1e04480}
Note how cp->first is mode DImode.
Now assume that all the real uses of cp->first occur as SUBREG
expressions. But there is a DImode clobber of cp->first. Like this:
(insn 7 2 3 2 (clobber (reg/v:DI 33 [ u ]))
"/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":404 -1
(nil))
(insn 3 7 4 2 (set (subreg:HI (reg/v:DI 33 [ u ]) 0)
(mem/c:HI (reg/f:HI 9 ap) [4 u+0 S2 A16]))
"/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":404 5 {*movhi_h8300}
(nil))
(insn 4 3 5 2 (set (subreg:HI (reg/v:DI 33 [ u ]) 2)
(mem/c:HI (plus:HI (reg/f:HI 9 ap)
(const_int 2 [0x2])) [4 u+2 S2 A16]))
"/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":404 5 {*movhi_h8300}
(nil))
[ ... ]
(insn 35 32 37 5 (parallel [
(set (reg:SI 39 [ _32 ])
(lshiftrt:SI (subreg:SI (reg/v:DI 33 [ u ]) 0)
(subreg:QI (reg:HI 38) 1)))
(clobber (scratch:QI))
]) "/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":415 229 {*shiftsi}
(expr_list:REG_DEAD (reg:HI 38)
(expr_list:REG_DEAD (reg/v:DI 33 [ u ])
(expr_list:REG_EQUIV (mem/j/c:SI (plus:HI (reg/f:HI 11 fp)
(const_int -4 [0xfffc])) [1
w.s.low+0 S4 A16])
(nil)
There's other references to (reg 33), but again, they all use subregs.
The only real DImode reference to (reg 33) is in the clobber. And
remember that (reg 33) is involved in a copy chain.
So we'll eventually call allocno_copy_cost_saving and try to compute a
cost savings using:
2764 cost += cp->freq *
ira_register_move_cost[allocno_mode][rclass][rclass];
But ira_register_move_cost[DImode] is NULL -- it's never been
initialized, presumably because we never see a real DImode reference
to anything except in CLOBBER statements.
We can fix this in scan_one_insn via the attached patch. I'm not sure
if this is the best place to catch this or not.
I haven't included a testcase as this trips just building libgcc on
the H8 target. I could easily reduce it if folks think its worth the
trouble.
I've verified this allows libgcc to build on the H8 target and
bootstrapped/regression tested the change on x86_64-unknown-linux-gnu
as well.
Vlad, is this OK for the trunk, or should we be catching this elsewhere?
There is no harm to put this fix. We could check initialization at
every usage but it will be a big impact on IRA's speed. Another place
would be at the beginning of find_costs_and_classes in the following loop:
for (i = max_reg_num () - 1; i >= FIRST_PSEUDO_REGISTER; i--)
regno_best_class[i] = NO_REGS;
Still your place will save CPU cycles most probably.
So the patch is ok for me. Thank you, Jeff.
[PATCH][RFA][target/79404] Fix uninitialized reference to ira_register_move_cost[mode]
So imagine we have two allocnos related by a copy chain (two operand
architecture).
(gdb) p *cp->first
$11 = {num = 9, regno = 33, mode = DImode, wmode = DImode, aclass =
GENERAL_REGS, dont_reassign_p = 0,
bad_spill_p = 0, assigned_p = 1, conflict_vec_p = 0, hard_regno = -1,
next_regno_allocno = 0x0,
loop_tree_node = 0x1e0b190, nrefs = 13, freq = 8069, class_cost =
1380, updated_class_cost = 1380,
memory_cost = 29656, updated_memory_cost = 29656,
excess_pressure_points_num = 17, allocno_prefs = 0x0,
allocno_copies = 0x1e4b400, cap = 0x0, cap_member = 0x0, num_objects
= 1, objects = {0x1e8b6a0, 0x0},
call_freq = 0, calls_crossed_num = 0, cheap_calls_crossed_num = 0,
crossed_calls_clobbered_regs = 0,
hard_reg_costs = 0x1da9510, updated_hard_reg_costs = 0x0,
conflict_hard_reg_costs = 0x0,
updated_conflict_hard_reg_costs = 0x0, add_data = 0x1e04378}
(gdb) p *cp->second
$12 = {num = 12, regno = 39, mode = SImode, wmode = SImode, aclass =
GENERAL_REGS, dont_reassign_p = 0,
bad_spill_p = 1, assigned_p = 1, conflict_vec_p = 0, hard_regno = 2,
next_regno_allocno = 0x0,
loop_tree_node = 0x1e0b190, nrefs = 2, freq = 388, class_cost = 0,
updated_class_cost = 0, memory_cost = 1552,
updated_memory_cost = 1552, excess_pressure_points_num = 0,
allocno_prefs = 0x0, allocno_copies = 0x1e4b400,
cap = 0x0, cap_member = 0x0, num_objects = 2, objects = {0x1e8b7e0,
0x1e8b830}, call_freq = 0,
calls_crossed_num = 0, cheap_calls_crossed_num = 0,
crossed_calls_clobbered_regs = 0,
hard_reg_costs = 0x1da9550, updated_hard_reg_costs = 0x0,
conflict_hard_reg_costs = 0x0,
updated_conflict_hard_reg_costs = 0x0, add_data = 0x1e04480}
Note how cp->first is mode DImode.
Now assume that all the real uses of cp->first occur as SUBREG
expressions. But there is a DImode clobber of cp->first. Like this:
(insn 7 2 3 2 (clobber (reg/v:DI 33 [ u ]))
"/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":404 -1
(nil))
(insn 3 7 4 2 (set (subreg:HI (reg/v:DI 33 [ u ]) 0)
(mem/c:HI (reg/f:HI 9 ap) [4 u+0 S2 A16]))
"/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":404 5 {*movhi_h8300}
(nil))
(insn 4 3 5 2 (set (subreg:HI (reg/v:DI 33 [ u ]) 2)
(mem/c:HI (plus:HI (reg/f:HI 9 ap)
(const_int 2 [0x2])) [4 u+2 S2 A16]))
"/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":404 5 {*movhi_h8300}
(nil))
[ ... ]
(insn 35 32 37 5 (parallel [
(set (reg:SI 39 [ _32 ])
(lshiftrt:SI (subreg:SI (reg/v:DI 33 [ u ]) 0)
(subreg:QI (reg:HI 38) 1)))
(clobber (scratch:QI))
]) "/home/gcc/GIT-2/gcc/libgcc/libgcc2.c":415 229 {*shiftsi}
(expr_list:REG_DEAD (reg:HI 38)
(expr_list:REG_DEAD (reg/v:DI 33 [ u ])
(expr_list:REG_EQUIV (mem/j/c:SI (plus:HI (reg/f:HI 11 fp)
(const_int -4 [0xfffc])) [1
w.s.low+0 S4 A16])
(nil)
There's other references to (reg 33), but again, they all use subregs.
The only real DImode reference to (reg 33) is in the clobber. And
remember that (reg 33) is involved in a copy chain.
So we'll eventually call allocno_copy_cost_saving and try to compute a
cost savings using:
2764 cost += cp->freq *
ira_register_move_cost[allocno_mode][rclass][rclass];
But ira_register_move_cost[DImode] is NULL -- it's never been
initialized, presumably because we never see a real DImode reference to
anything except in CLOBBER statements.
We can fix this in scan_one_insn via the attached patch. I'm not sure
if this is the best place to catch this or not.
I haven't included a testcase as this trips just building libgcc on the
H8 target. I could easily reduce it if folks think its worth the trouble.
I've verified this allows libgcc to build on the H8 target and
bootstrapped/regression tested the change on x86_64-unknown-linux-gnu as
well.
Vlad, is this OK for the trunk, or should we be catching this elsewhere?
Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index f352051..2170e57 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2017-02-13 Jeff Law
+
+ PR target/79404
+ * ira-costs.c (scan_one_insn): Initialize register move costs
+ for pseudos seen in USE/CLOBBER insns.
+
2017-02-13 Aaron Sawdey
PR target/79449
diff --git a/gcc/ira-costs.c b/gcc/ira-costs.c
index c561db6..1737430 100644
--- a/gcc/ira-costs.c
+++ b/gcc/ira-costs.c
@@ -1438,9 +1438,25 @@ scan_one_insn (rtx_insn *insn)
return insn;
pat_code = GET_CODE (PATTERN (insn));
- if (pat_code == USE || pat_code == CLOBBER || pat_code == ASM_INPUT)
+ if (pat_code == ASM_INPUT)
return insn;
+ /* If INSN is a USE/CLOBBER of a pseudo in a mode M then go ahead
+ and initialize the register move costs of mode M.
+
+ The pseudo may be related to another pseudo via a copy (implicit or
+ explicit) and if there are no mode M uses/sets of the original
+
