https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98289
Jorn Wolfgang Rennecke <amylaar at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |---
CC| |amylaar at gcc dot gnu.org
--- Comment #6 from Jorn Wolfgang Rennecke <amylaar at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> --- gcc/shrink-wrap.c.jj 2020-07-28 15:39:09.983756571 +0200
> +++ gcc/shrink-wrap.c 2020-12-15 19:15:00.213861334 +0100
> @@ -494,7 +494,7 @@ can_get_prologue (basic_block pro, HARD_
> edge e;
> edge_iterator ei;
> FOR_EACH_EDGE (e, ei, pro->preds)
> - if (e->flags & (EDGE_COMPLEX | EDGE_CROSSING)
> + if (e->flags & EDGE_COMPLEX
> && !dominated_by_p (CDI_DOMINATORS, e->src, pro))
> return false;
>
> fixes it for me. Not sure I understand why EDGE_CROSSING has been listed
> there,
> if pro is in the cold partition and has EDGE_CROSSING edge leading to it,
> then
> why can't the prologue be added to the cold partition and the jump
> redirected to it?
EDGE_CROSSING jumps may not reach when you use a label as the source on targets
where
HAS_LONG_UNCOND_BRANCH is false. I've investigated why this doesn't cause an
ICE on
RISC-V, and found that that's due to another bug in
ira.c:indirect_jump_optimize .
When fix_crossing_unconditional_branches emits a load of a label, the RISC-V
port
puts this basically into SSA form, like:
(insn 80 62 81 12 (set (reg:SI 150)
(high:SI (label_ref:SI 45))) 276 {*movsi_internal}
(insn_list:REG_LABEL_OPERAND 45 (nil)))
(insn 81 80 82 12 (set (reg:SI 149)
(lo_sum:SI (reg:SI 150)
(label_ref:SI 45))) 270 {*lowsi}
(expr_list:REG_DEAD (reg:SI 150)
(insn_list:REG_LABEL_OPERAND 45 (expr_list:REG_EQUAL (label_ref:SI 45)
(nil)))))
(jump_insn/j 82 81 63 12 (set (pc)
(reg:SI 149)) 418 {indirect_jumpsi}
(expr_list:REG_DEAD (reg:SI 149)
(nil))
-> 45)
with a REG_EQUAL note on insn 81 to show what (reg:SI 149) is set to.
which allows indirect_jump_optimize to easily find the def_insn and replace the
register with the label.
gdb) call debug(bb)
(code_label 68 67 70 14 37 (nil) [1 uses])
(note 70 68 88 14 [bb 14] NOTE_INSN_BASIC_BLOCK)
(insn 88 70 89 14 (set (reg:SI 154)
(high:SI (label_ref:SI 21))) 276 {*movsi_internal}
(insn_list:REG_LABEL_OPERAND 21 (nil)))
(insn 89 88 90 14 (set (reg:SI 153)
(lo_sum:SI (reg:SI 154)
(label_ref:SI 21))) 270 {*lowsi}
(expr_list:REG_DEAD (reg:SI 154)
(insn_list:REG_LABEL_OPERAND 21 (expr_list:REG_EQUAL (label_ref:SI 21)
(nil)))))
(jump_insn/j 90 89 71 14 (set (pc)
(label_ref:SI 21)) 417 {jump}
(expr_list:REG_DEAD (reg:SI 153)
(nil))
-> 21)
(gdb) call debug(bb->succs)
[0] = <edge 0x0x7fffe9fb70f0 (14 -> 8)>
(gdb) call debug((edge_def*)0x7fffe9fb70f0)
<edge (14 -> 8)>
14 [always] (CROSSING)
Which is wrong, because RISC-V is a port with HAS_LONG_UNCOND_BRANCH == false.
You should not rely on the ira bug to paper over the problem here. Either
reject
indirect jumps if !HAS_LONG_UNCOND_BRANCH , or teach patch_jump_insn how to
redirect an indirect jump that uses a register that dies in this jump,
by changing the load (which is typically more than one insn), or overriding it
with
a new one. And then there might still be some cases you have to reject because
the
register doesn't die in the jump and you can't tell how to make this safe.
