From: Kyrylo Tkachov <[email protected]>
gen_call_used_regs_seq adds the registers it zeroes to
crtl->must_be_zero_on_return. df_epilogue_uses_p reports those registers as
used by the epilogue, so they join the exit block use set. That set in turn
decides which registers a sibling call is recorded as clobbering:
df_get_call_refs skips the clobber def of any register that the exit block
uses. Growing the set therefore invalidates the recorded defs of every
sibling call in the function, but pass_zero_call_used_regs only refreshed the
artificial refs, through df_update_exit_block_uses.
To hit this a function needs one return path that is a sibling call and
another that is a plain return. The zeroing sequence goes on the plain
return path while the sibling call keeps its stale defs. The next pass to
run df_analyze then ICEs in a df checking compiler. On aarch64 that pass is
narrow_gp_writes:
during RTL pass: narrow_gp_writes
internal compiler error: in df_refs_verify, at df-scan.cc:4012
0x87315f df_refs_verify
0xed6b66 df_insn_refs_verify
0xed8b63 df_bb_verify
0xed8ee7 df_scan_verify()
0xec2161 df_verify()
For the sibling call insn the recorded chain holds 65 defs while a fresh scan
collects 57, the difference being x0 to x7:
(call_insn/j:TI 82 147 83 5 (parallel [
(call (mem:DI (symbol_ref:DI ("bar")))
(const_int 0 [0]))
(unspec:DI [(const_int 2 [0x2])] UNSPEC_CALLEE_ABI)
(return)
]) 110 {*sibcall_insn}
recorded defs { 0 1 2 3 4 5 6 7 8 ... 18 32 ... 86 16 17 }
fresh defs { 8 ... 18 32 ... 86 16 17 }
Call df_update_entry_exit_and_calls once after all return blocks have been
handled. This is the same fix that PR rtl-optimization/78400 applied to
shrink-wrapping, and thread_prologue_and_epilogue_insns already does it for
the same reason. The df_update_exit_block_uses call being replaced came from
PR middle-end/100775, which covered only the artificial refs of the exit
block.
df_compute_regs_ever_live also calls df_update_entry_exit_and_calls whenever
regs_ever_live changed, which is why the bug usually hides: when zeroing a
register makes it ever-live for the first time, the next df_analyze repairs
the refs by accident. The testcase uses -fzero-call-used-regs=used-arg so
that every zeroed register is already live.
The original report used -finstrument-functions-once, which supplies the two
return paths through the tail call to __cyg_profile_func_exit.
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?
Thanks,
KYrill
gcc/ChangeLog:
PR rtl-optimization/127445
* function.cc (gen_call_used_regs_seq): Return whether a zeroing
sequence was emitted. Do not update the exit block uses here.
(pass_zero_call_used_regs::execute): Call
df_update_entry_exit_and_calls once when a sequence was emitted.
gcc/testsuite/ChangeLog:
PR rtl-optimization/127445
* gcc.target/aarch64/pr127445.c: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/function.cc | 47 ++++++++++++---------
gcc/testsuite/gcc.target/aarch64/pr127445.c | 23 ++++++++++
2 files changed, 51 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr127445.c
diff --git a/gcc/function.cc b/gcc/function.cc
index fd9da3003cd..60031f93ea5 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -5898,9 +5898,11 @@ make_prologue_seq (void)
}
/* Emit a sequence of insns to zero the call-used registers before RET
- according to ZERO_REGS_TYPE. */
+ according to ZERO_REGS_TYPE. Return true if a sequence was emitted, in
+ which case crtl->must_be_zero_on_return has grown and the caller has to
+ refresh the data flow information. */
-static void
+static bool
gen_call_used_regs_seq (rtx_insn *ret, unsigned int zero_regs_type)
{
bool only_gpr = true;
@@ -5909,12 +5911,12 @@ gen_call_used_regs_seq (rtx_insn *ret, unsigned int
zero_regs_type)
/* No need to zero call-used-regs in main (). */
if (MAIN_NAME_P (DECL_NAME (current_function_decl)))
- return;
+ return false;
/* No need to zero call-used-regs if __builtin_eh_return is called
since it isn't a normal function return. */
if (crtl->calls_eh_return)
- return;
+ return false;
/* If only_gpr is true, only zero call-used registers that are
general-purpose registers; if only_used is true, only zero
@@ -5978,7 +5980,7 @@ gen_call_used_regs_seq (rtx_insn *ret, unsigned int
zero_regs_type)
}
if (hard_reg_set_empty_p (selected_hardregs))
- return;
+ return false;
/* Now that we have a hard register set that needs to be zeroed, pass it to
target to generate zeroing sequence. */
@@ -5995,21 +5997,20 @@ gen_call_used_regs_seq (rtx_insn *ret, unsigned int
zero_regs_type)
gcc_assert (hard_reg_set_subset_p (zeroed_hardregs, all_call_used_regs));
rtx_insn *seq = end_sequence ();
- if (seq)
- {
- /* Emit the memory blockage and register clobber asm volatile before
- the whole sequence. */
- start_sequence ();
- expand_asm_reg_clobber_mem_blockage (zeroed_hardregs);
- rtx_insn *seq_barrier = end_sequence ();
+ if (!seq)
+ return false;
- emit_insn_before (seq_barrier, ret);
- emit_insn_before (seq, ret);
+ /* Emit the memory blockage and register clobber asm volatile before
+ the whole sequence. */
+ start_sequence ();
+ expand_asm_reg_clobber_mem_blockage (zeroed_hardregs);
+ rtx_insn *seq_barrier = end_sequence ();
- /* Update the data flow information. */
- crtl->must_be_zero_on_return |= zeroed_hardregs;
- df_update_exit_block_uses ();
- }
+ emit_insn_before (seq_barrier, ret);
+ emit_insn_before (seq, ret);
+
+ crtl->must_be_zero_on_return |= zeroed_hardregs;
+ return true;
}
@@ -6836,13 +6837,21 @@ pass_zero_call_used_regs::execute (function *fun)
/* Iterate over the function's return instructions and insert any
register zeroing required by the -fzero-call-used-regs command-line
option or the "zero_call_used_regs" function attribute. */
+ bool zeroed = false;
FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
{
rtx_insn *insn = BB_END (e->src);
if (JUMP_P (insn) && ANY_RETURN_P (JUMP_LABEL (insn)))
- gen_call_used_regs_seq (insn, zero_regs_type);
+ zeroed |= gen_call_used_regs_seq (insn, zero_regs_type);
}
+ /* The registers in must_be_zero_on_return are uses of the exit block, see
+ df_epilogue_uses_p. Growing that set also changes which registers a
+ sibling call is recorded as clobbering, see df_get_call_refs, so the
+ calls have to be rescanned as well. */
+ if (zeroed)
+ df_update_entry_exit_and_calls ();
+
return 0;
}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr127445.c
b/gcc/testsuite/gcc.target/aarch64/pr127445.c
new file mode 100644
index 00000000000..cc0974c9180
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr127445.c
@@ -0,0 +1,23 @@
+/* PR rtl-optimization/127445 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mnarrow-gp-writes -fzero-call-used-regs=used-arg" } */
+
+/* One return path of foo is a sibling call and the other is a plain return.
+ The zeroing sequence emitted on the plain return path adds the argument
+ registers to the exit block use set, which changes the set of registers
+ that the sibling call is recorded as clobbering. used-arg only zeroes
+ registers that are already live, so nothing else forces a rescan of the
+ call to keep its data flow information correct.
+
+ The stale refs are only diagnosed by a compiler configured with
+ --enable-checking=df, which verifies them in df_analyze. */
+
+int bar (int, int, int, int, int, int, int, int);
+
+int
+foo (int a, int b, int c, int d, int e, int f, int g, int h)
+{
+ if (a)
+ return bar (a, b, c, d, e, f, g, h);
+ return a + b + c + d + e + f + g + h;
+}
--
2.50.1 (Apple Git-155)
