The bit-insert sequences generated by store_bit_field can clobber hard
registers (such as the flags register on x86) as a side effect. If
such a register is live at the insertion point, the transformation
would corrupt it, breaking flag-dependent sequences like carry chains
(see PR119795).
Add a liveness check in process_store_forwarding: after generating the
bit-insert sequences, collect the hard registers they clobber (excluding
the intended destination) and reject the transformation if any of them
is live at the insertion point. Per-insn live-out hard-register sets
are computed once per BB by simulating it backward with
df_simulate_one_insn_backwards, and cached in store_forwarding_analyzer
so subsequent forwarding candidates in the same BB reuse the result.
The cache is populated lazily on the first candidate that produces
non-empty clobbers, so on targets where bit-inserts have no side-effect
clobbers (such as aarch64 bfi) the BB walk never runs.
gcc/ChangeLog:
* avoid-store-forwarding.cc: Include regs.h.
(record_hard_reg_clobbers): New callback.
(store_forwarding_analyzer::m_bb_live_after): New cache.
(store_forwarding_analyzer::compute_bb_live_after): New helper.
(store_forwarding_analyzer::process_store_forwarding): Add
liveness check for hard registers clobbered by bit-insert
sequences, using the cached per-BB live-out information, and
evict the load_insn entry from the cache before delete_insn
in the load-elim path.
(store_forwarding_analyzer::avoid_store_forwarding): Clear the
per-BB liveness cache on entry.
Signed-off-by: Konstantinos Eleftheriou <[email protected]>
---
gcc/avoid-store-forwarding.cc | 73 ++++++++++++++++++++++++++++++++++-
1 file changed, 72 insertions(+), 1 deletion(-)
diff --git a/gcc/avoid-store-forwarding.cc b/gcc/avoid-store-forwarding.cc
index 83abdd49ba9c..0c065ba1a75a 100644
--- a/gcc/avoid-store-forwarding.cc
+++ b/gcc/avoid-store-forwarding.cc
@@ -34,6 +34,7 @@
#include "expmed.h"
#include "recog.h"
#include "regset.h"
+#include "regs.h"
#include "df.h"
#include "expr.h"
#include "memmodel.h"
@@ -101,6 +102,15 @@ public:
rtx load_mem);
void avoid_store_forwarding (basic_block);
void update_stats (function *);
+
+private:
+ /* Per-insn live-out hard-register sets for the current BB. Populated
+ lazily on the first candidate with bit-insert side-effect clobbers
+ (so aarch64 bfi pays nothing). Cleared on each avoid_store_forwarding
+ entry. */
+ hash_map<rtx_insn *, HARD_REG_SET> m_bb_live_after;
+
+ void compute_bb_live_after (basic_block bb);
};
/* Return a bit insertion sequence that would make DEST have the correct value
@@ -134,6 +144,35 @@ generate_bit_insert_sequence (store_fwd_info *store_info,
rtx dest)
return insns;
}
+/* note_stores callback: record hard regs clobbered (not set) by an insn,
+ to capture side-effect clobbers (e.g. flags) without the intended dest. */
+
+static void
+record_hard_reg_clobbers (rtx x, const_rtx pat, void *data)
+{
+ if (GET_CODE (pat) == CLOBBER && REG_P (x) && HARD_REGISTER_P (x))
+ add_to_hard_reg_set ((HARD_REG_SET *) data, GET_MODE (x), REGNO (x));
+}
+
+/* Populate m_bb_live_after with the hard registers live immediately
+ after each real insn in BB. */
+
+void
+store_forwarding_analyzer::compute_bb_live_after (basic_block bb)
+{
+ auto_bitmap live;
+ df_simulate_initialize_backwards (bb, live);
+ rtx_insn *scan;
+ FOR_BB_INSNS_REVERSE (bb, scan)
+ if (INSN_P (scan))
+ {
+ HARD_REG_SET hrs;
+ REG_SET_TO_HARD_REG_SET (hrs, live);
+ m_bb_live_after.put (scan, hrs);
+ df_simulate_one_insn_backwards (bb, scan, live);
+ }
+}
+
/* Return true iff a store to STORE_MEM would write to a sub-region of bytes
from what LOAD_MEM would read. If true also store the relative byte offset
of the store within the load to OFF_VAL. */
@@ -332,6 +371,32 @@ process_store_forwarding (vec<store_fwd_info> &stores,
rtx_insn *load_insn,
it->store_saved_value_insn = insn2;
}
+ /* Reject if the bit-insert sequences clobber a hard register live at
+ the insertion point (e.g. shift/and/or on x86 clobber flags, which
+ would break carry chains). Done before the target cost query so
+ we skip cost work on candidates we would reject anyway. */
+ HARD_REG_SET clobbered_regs;
+ CLEAR_HARD_REG_SET (clobbered_regs);
+ FOR_EACH_VEC_ELT (stores, i, it)
+ for (rtx_insn *ins = it->bits_insert_insns; ins; ins = NEXT_INSN (ins))
+ note_stores (ins, record_hard_reg_clobbers, &clobbered_regs);
+
+ if (!hard_reg_set_empty_p (clobbered_regs))
+ {
+ if (m_bb_live_after.is_empty ())
+ compute_bb_live_after (BLOCK_FOR_INSN (load_insn));
+
+ const HARD_REG_SET *live_at_insert = m_bb_live_after.get (load_insn);
+ if (live_at_insert
+ && hard_reg_set_intersect_p (clobbered_regs, *live_at_insert))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Not transformed: bit-insert clobbers live hard reg.\n");
+ return false;
+ }
+ }
+
if (load_elim)
total_cost -= insn_cost (load_insn, true);
@@ -421,7 +486,11 @@ process_store_forwarding (vec<store_fwd_info> &stores,
rtx_insn *load_insn,
df_insn_rescan (load_insn);
if (load_elim)
- delete_insn (load_insn);
+ {
+ /* Prevent a dangling rtx_insn * key after delete_insn. */
+ m_bb_live_after.remove (load_insn);
+ delete_insn (load_insn);
+ }
return true;
}
@@ -434,6 +503,8 @@ store_forwarding_analyzer::avoid_store_forwarding
(basic_block bb)
if (!optimize_bb_for_speed_p (bb))
return;
+ m_bb_live_after.empty ();
+
auto_vec<store_fwd_info, 8> store_exprs;
rtx_insn *insn;
unsigned int insn_cnt = 0;
--
2.52.0
