https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125375
Richard Sandiford <rsandifo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rsandifo at gcc dot gnu.org
--- Comment #19 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
I think we need to be more precise about what "B is a forwarder block" means in
an RTL context. Does it mean "an edge to B can be redirected to an edge to
B'"? Or does it mean "B has no effect other than to jump to B'"? A block of
the form:
(clobber X)
(set (pc) (label_ref L))
meets the first definition but not the second. And that's true regardless of
what X is.
When redirecting a jump to a different location, such as:
Edge 4->15 redirected to 18
in this testcase, the first definition is the correct one. The new edge from
4->18 drops the clobber, but that's ok. At worst it means less accurate
lifetime information, but it is clearly an improvement.
The same holds true for the second optimisation:
Edge 16->15 redirected to 18
But we also have:
Forwarding edge 14->15 to 18 failed.
Because of that failure, we have:
- block 13, which has the well-defined return value, jumping directly
to block 18
- block 14, which does not have a well-defined return value, jumping
to block 15, which has a clobber of ax but is nevertheless treated
as forwarding to block 18
Block 13 and block 14 have trailing instructions in common. And cross-jumping
thinks that a jump to B' can be merged with a forwarder to B'. In other words,
it assumes the second definition above:
/* Search backward through forwarder blocks. We don't need to worry
about multiple entry or chained forwarders, as they will be optimized
away. We do this to look past the unconditional jump following a
conditional jump that is required due to the current CFG shape. */
if (single_pred_p (src1)
&& FORWARDER_BLOCK_P (src1))
e1 = single_pred_edge (src1), src1 = e1->src;
if (single_pred_p (src2)
&& FORWARDER_BLOCK_P (src2))
e2 = single_pred_edge (src2), src2 = e2->src;
So either we need to separate the concept of forwarder block into two, or we
need to treat all clobbers as flow_active_insn_p. The latter seems simpler,
but it might lead to less jump threading:
diff --git a/gcc/cfgrtl.cc b/gcc/cfgrtl.cc
index 0ec72f08fa8..089f34e7f80 100644
--- a/gcc/cfgrtl.cc
+++ b/gcc/cfgrtl.cc
@@ -567,16 +567,13 @@ flow_active_insn_p (const rtx_insn *insn)
if (active_insn_p (insn))
return true;
- /* A clobber of the function return value exists for buggy
- programs that fail to return a value. Its effect is to
- keep the return value from being live across the entire
- function. If we allow it to be skipped, we introduce the
- possibility for register lifetime confusion.
- Similarly, keep a USE of the function return value, otherwise
+ if (GET_CODE (PATTERN (insn)) == CLOBBER)
+ return true;
+
+ /* Keep a USE of the function return value, otherwise
the USE is dropped and we could fail to thread jump if USE
appears on some paths and not on others, see PR90257. */
- if ((GET_CODE (PATTERN (insn)) == CLOBBER
- || GET_CODE (PATTERN (insn)) == USE)
+ if (GET_CODE (PATTERN (insn)) == USE
&& REG_P (XEXP (PATTERN (insn), 0))
&& REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))
return true;
I'll give that a try to see what the fallout is.
