On Fri, Apr 04, 2025 at 01:56:31PM +0100, Richard Sandiford wrote:
> My suggestion isn't elegant, but: once we've decided not to delete
> an instruction, we need to keep all definitions of all registers used
> by the instruction (given the way that the algorithm works). So how
> about instead just iterating over DF_INSN_USES in the else arm of:
>
> if (! live_insn && dbg_cnt (delete_trivial_dead))
>
> and incrementing counts for each register listed? That will
> double-count in some cases, but only once we've decided to keep
> (all definitions of) a register. And nothing should rely on the
> final counts being zero.
I think incrementing everything is unnecessary, there we could easily also
overflow the counters quickly.
But just incrementing the self-references might be good enough.
So what about the following patch then? Fixes the testcase too.
Or shall we go with Eric's suggestion?
And for both possibilities, without or with the extra
else if (GET_CODE (PATTERN (x)) == PARALLEL) hunk from my previous version
of the patch with the nsets stuff removed (i.e. ensure dest is pc_rtx
whenever the insn is certainly never removable)?
2025-04-04 Jakub Jelinek <ja...@redhat.com>
PR rtl-optimization/119594
* cse.cc (count_reg_usage): If INCR is 0, increment by 1 counts
of encountered REGs equal to dest rather than incrementing by INCR
counts of REGs not equal to dest.
(delete_trivially_dead_insns): For kept non-DEBUG instructions
call count_reg_usage with INCR 0.
* gcc.dg/pr119594.c: New test.
--- gcc/cse.cc.jj 2025-04-03 13:17:56.506158871 +0200
+++ gcc/cse.cc 2025-04-04 15:28:36.675315825 +0200
@@ -6755,7 +6755,9 @@ cse_main (rtx_insn *f ATTRIBUTE_UNUSED,
/* Count the number of times registers are used (not set) in X.
COUNTS is an array in which we accumulate the count, INCR is how much
- we count each register usage.
+ we count each register usage. INCR 0 has special meaning to increment
+ by 1 only registers which have not been incremented because they appear
+ in a SET_SRC of a SET which sets the same register.
Don't count a usage of DEST, which is the SET_DEST of a SET which
contains X in its SET_SRC. This is because such a SET does not
@@ -6778,7 +6780,12 @@ count_reg_usage (rtx x, int *counts, rtx
switch (code = GET_CODE (x))
{
case REG:
- if (x != dest)
+ if (incr == 0)
+ {
+ if (x == dest)
+ ++counts[REGNO (x)];
+ }
+ else if (x != dest)
counts[REGNO (x)] += incr;
return;
@@ -7143,6 +7150,11 @@ delete_trivially_dead_insns (rtx_insn *i
later_debug_set_vars.safe_push (INSN_VAR_LOCATION_DECL (insn));
TREE_VISITED (INSN_VAR_LOCATION_DECL (insn)) = 1;
}
+ /* If we keep an instruction, make sure to increment even the
+ usages of regs in SET_SRC when SET_DEST is the same register.
+ See PR119594. */
+ if (!DEBUG_INSN_P (insn))
+ count_reg_usage (insn, counts, NULL_RTX, 0);
}
}
--- gcc/testsuite/gcc.dg/pr119594.c.jj 2025-04-03 14:19:35.444575677 +0200
+++ gcc/testsuite/gcc.dg/pr119594.c 2025-04-03 14:19:13.962871101 +0200
@@ -0,0 +1,32 @@
+/* PR rtl-optimization/119594 */
+/* { dg-do run } */
+/* { dg-options "-Os -fno-dce -fno-tree-dce -fno-tree-dse" } */
+
+int a, b;
+static unsigned c = -1;
+
+void
+foo (int e)
+{
+ a = a ^ e;
+}
+
+void
+bar (long e)
+{
+ foo (e >> 1);
+}
+
+int
+main ()
+{
+ int g[2];
+ for (int h = 0; h < 2; h++)
+ g[h] = -1;
+ for (; b; b++)
+ ;
+ g[1] = 0;
+ bar (c);
+ if (!a)
+ __builtin_abort ();
+}
Jakub
