Hi!
The following testcase is miscompiled by delete_trivially_dead_insns,
latently since r0-6313, actually since r15-1575.
The problem is in that r0-6313 change, which made count_reg_usage not
count uses of the pseudo which the containing SET sets. That is needed
so we can delete those instructions as trivially dead if they are really
dead, but has the following problem. After fwprop proper we have:
(insn 7 2 8 2 (set (reg/v:DI 101 [ g ])
(const_int -1 [0xffffffffffffffff])) "pr119594.c":8:10 95
{*movdi_internal}
(nil))
...
(insn 26 24 27 7 (set (reg:DI 104 [ g ])
(zero_extend:DI (subreg:SI (reg/v:DI 101 [ g ]) 0))) "pr119594.c":11:8
175 {*zero_extendsidi2}
(expr_list:REG_EQUAL (const_int 4294967295 [0xffffffff])
(expr_list:REG_DEAD (reg/v:DI 101 [ g ])
(nil))))
(insn 27 26 28 7 (set (reg/v:DI 101 [ g ])
(zero_extend:DI (subreg:SI (reg/v:DI 101 [ g ]) 0))) "pr119594.c":11:8
175 {*zero_extendsidi2}
(expr_list:REG_EQUAL (const_int 4294967295 [0xffffffff])
(expr_list:REG_UNUSED (reg/v:DI 101 [ g ])
(nil))))
and nothing else uses or sets the 101 and 104 pseudos. The subpass doesn't
look at REG_UNUSED or REG_DEAD notes (correctly, as they aren't guaranteed
to be accurate). The last change in the IL was forward propagation of
(reg:DI 104 [ g ]) value into the following insn.
Now, count_reg_usage doesn't count anything on insn 7, the SET_DEST is a
reg, so we don't count that and SET_SRC doesn't contain any regs.
On insn 26 it counts one usage of pseudo 101 (so counts[101] = 1) and
on insn 27 since r0-6313 doesn't count anything as that insn sets
pseudo 101 to something that uses it, it isn't a side-effect instruction
and can't throw.
Now, after counting reg usages the subpass walks the IL from end to start,
sees insn 27, counts[101] is non-zero, so insn_live_p is true, nothing is
deleted. Then sees insn 26, counts[104] is zero, insn_live_p is false,
we delete the insn and decrease associated counts, in this case counts[101]
becomes zero. And finally later we process insn 7, counts[101] is now zero,
insn_live_p is false, we delete the insn (and decrease associated counts,
which aren't any).
Except that this resulted in insn 27 staying in the IL but using a REG
which is no longer set (and worse, having a REG_EQUAL note of something we
need later in the same bb, so we then assume pseudo 101 contains 0xffffffff,
which it no longer does.
Now, if insn 26 was after insn 27, this would work just fine, we'd first
delete that and then insn 27 and then insn 7, which is why most of the time
it happens to work fine.
The following patch attempts to detect these cases by counting the uses
of pseudos in instructions which set it in a separate new slice of the
counts array and then we can check if we've decreased usage count to zero
on something that still has such self-uses non-zero (so some such
instructions not removed) and do another pass of the IL and delete those
(and just those, without updating further reg usage counts so that we don't
have to repeat; also, I think we really don't need to update debug insns at
that point, in such cases it were originally multiple sets of the pseudo
(unless the code was undefined) and there is following loop on the IL to
adjust the further debug insns.
Previously, we forced (through passing dest = pc_rtx) counting the uses
even in sets of such pseudo on some instructions which would be never
insn_live_p regardless of counts array (insns throwing with non-call
exceptions and side_effects_p instructions). The patch extends it
to other cases where it is never insn_live_p regardless of counts array
content (e.g. insns which set hard registers in parallel with some
pseudo, etc.) and also add to that the case of setting 2 or more pseudos
in one instruction (something insn_live_p could return false if both
pseudos are unused but if we aren't lucky and one of them is, we would
need to keep it but could remove the setter of the self-reference use).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Note, we never care about usage counts of hard registers, so I wonder
(perhaps for GCC 16) if we shouldn't decrease the size of slices from
2 * nreg (or for -g 4 * nreg) to 2 * (nreg - FIRST_PSEUDO_REGISTER)
or 4 times that and simply only track pseudo registers with REGNO (x) -
FIRST_PSEUDO_REGISTER indexes.
2025-04-04 Jakub Jelinek <ja...@redhat.com>
PR rtl-optimization/119594
* cse.cc (count_reg_usage): Add NREG argument, pass it to
recursive call. For REG increment counts[REGNO (x) + nreg] if
x == dest and nreg is non-zero. For INSNs, set dest = pc_rtx
if PATTERN is a PARALLEL and insn_live_p will be true regardless
of count array content, or if it sets 2 or more pseudos.
(delete_trivially_dead_insns): For MAY_HAVE_DEBUG_BIND_INSNS allocate
4 nreg array slices instead of 3 and adjust count + nreg * 2 to
count + nreg * 3 and count + nreg to count + nreg * 2, for
!MAY_HAVE_DEBUG_BIND_INSNS allocate 2 nreg array slices. Adjust
count_reg_usage callers. If for any pseudo register counts[i]
is 0 and counts[i + nreg] isn't 0, walk the IL again and remove
insns which set such registers.
* gcc.dg/pr119594.c: New test.
--- gcc/cse.cc.jj 2025-04-03 13:17:56.506158871 +0200
+++ gcc/cse.cc 2025-04-03 14:09:59.804483513 +0200
@@ -6765,7 +6765,7 @@ cse_main (rtx_insn *f ATTRIBUTE_UNUSED,
deleted here. */
static void
-count_reg_usage (rtx x, int *counts, rtx dest, int incr)
+count_reg_usage (rtx x, int *counts, rtx dest, int incr, int nreg)
{
enum rtx_code code;
rtx note;
@@ -6780,6 +6780,8 @@ count_reg_usage (rtx x, int *counts, rtx
case REG:
if (x != dest)
counts[REGNO (x)] += incr;
+ else if (nreg)
+ counts[REGNO (x) + nreg] += incr;
return;
case PC:
@@ -6793,16 +6795,16 @@ count_reg_usage (rtx x, int *counts, rtx
/* If we are clobbering a MEM, mark any registers inside the address
as being used. */
if (MEM_P (XEXP (x, 0)))
- count_reg_usage (XEXP (XEXP (x, 0), 0), counts, NULL_RTX, incr);
+ count_reg_usage (XEXP (XEXP (x, 0), 0), counts, NULL_RTX, incr, nreg);
return;
case SET:
/* Unless we are setting a REG, count everything in SET_DEST. */
if (!REG_P (SET_DEST (x)))
- count_reg_usage (SET_DEST (x), counts, NULL_RTX, incr);
+ count_reg_usage (SET_DEST (x), counts, NULL_RTX, incr, nreg);
count_reg_usage (SET_SRC (x), counts,
dest ? dest : SET_DEST (x),
- incr);
+ incr, nreg);
return;
case DEBUG_INSN:
@@ -6817,9 +6819,45 @@ count_reg_usage (rtx x, int *counts, rtx
if ((!cfun->can_delete_dead_exceptions && !insn_nothrow_p (x))
|| side_effects_p (PATTERN (x)))
dest = pc_rtx;
+ else if (GET_CODE (PATTERN (x)) == PARALLEL)
+ {
+ /* Similarly for PARALLEL, set DEST to pc_rtx for patterns
+ which will be always insn_live_p regardless of the content
+ of counts array. Those are also never considered trivially
+ dead. And finally set it to pc_rtx also for insns which set
+ multiple pseudo registers. While we might be able to remove
+ those sometimes, sometimes we might not and in that case
+ if it uses one of the SET_DESTs in the SET_SRC and it is
+ not a noop set, we could remove the setter of the SET_SRC
+ but keep the user in the IL. */
+ int nsets = 0;
+ for (i = XVECLEN (PATTERN (x), 0) - 1; i >= 0; i--)
+ {
+ rtx elt = XVECEXP (PATTERN (x), 0, i);
+
+ if (GET_CODE (elt) == SET)
+ {
+ if (set_noop_p (elt))
+ continue;
+ if (!REG_P (SET_DEST (elt))
+ || REGNO (SET_DEST (elt)) < FIRST_PSEUDO_REGISTER
+ || ++nsets > 1)
+ {
+ dest = pc_rtx;
+ break;
+ }
+ }
+ else if (GET_CODE (elt) != CLOBBER && GET_CODE (elt) != USE)
+ {
+ dest = pc_rtx;
+ break;
+ }
+ }
+ }
if (code == CALL_INSN)
- count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, dest, incr);
- count_reg_usage (PATTERN (x), counts, dest, incr);
+ count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, dest, incr,
+ nreg);
+ count_reg_usage (PATTERN (x), counts, dest, incr, nreg);
/* Things used in a REG_EQUAL note aren't dead since loop may try to
use them. */
@@ -6834,12 +6872,12 @@ count_reg_usage (rtx x, int *counts, rtx
Process all the arguments. */
do
{
- count_reg_usage (XEXP (eqv, 0), counts, dest, incr);
+ count_reg_usage (XEXP (eqv, 0), counts, dest, incr, nreg);
eqv = XEXP (eqv, 1);
}
while (eqv && GET_CODE (eqv) == EXPR_LIST);
else
- count_reg_usage (eqv, counts, dest, incr);
+ count_reg_usage (eqv, counts, dest, incr, nreg);
}
return;
@@ -6849,15 +6887,15 @@ count_reg_usage (rtx x, int *counts, rtx
/* FUNCTION_USAGE expression lists may include (CLOBBER (mem /u)),
involving registers in the address. */
|| GET_CODE (XEXP (x, 0)) == CLOBBER)
- count_reg_usage (XEXP (x, 0), counts, NULL_RTX, incr);
+ count_reg_usage (XEXP (x, 0), counts, NULL_RTX, incr, nreg);
- count_reg_usage (XEXP (x, 1), counts, NULL_RTX, incr);
+ count_reg_usage (XEXP (x, 1), counts, NULL_RTX, incr, nreg);
return;
case ASM_OPERANDS:
/* Iterate over just the inputs, not the constraints as well. */
for (i = ASM_OPERANDS_INPUT_LENGTH (x) - 1; i >= 0; i--)
- count_reg_usage (ASM_OPERANDS_INPUT (x, i), counts, dest, incr);
+ count_reg_usage (ASM_OPERANDS_INPUT (x, i), counts, dest, incr, nreg);
return;
case INSN_LIST:
@@ -6872,10 +6910,10 @@ count_reg_usage (rtx x, int *counts, rtx
for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
{
if (fmt[i] == 'e')
- count_reg_usage (XEXP (x, i), counts, dest, incr);
+ count_reg_usage (XEXP (x, i), counts, dest, incr, nreg);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
- count_reg_usage (XVECEXP (x, i, j), counts, dest, incr);
+ count_reg_usage (XVECEXP (x, i, j), counts, dest, incr, nreg);
}
}
@@ -7019,33 +7057,36 @@ delete_trivially_dead_insns (rtx_insn *i
/* First count the number of times each register is used. */
if (MAY_HAVE_DEBUG_BIND_INSNS)
{
- counts = XCNEWVEC (int, nreg * 3);
+ counts = XCNEWVEC (int, nreg * 4);
for (insn = insns; insn; insn = NEXT_INSN (insn))
if (DEBUG_BIND_INSN_P (insn))
{
- count_reg_usage (INSN_VAR_LOCATION_LOC (insn), counts + nreg,
- NULL_RTX, 1);
+ count_reg_usage (INSN_VAR_LOCATION_LOC (insn), counts + nreg * 2,
+ NULL_RTX, 1, 0);
TREE_VISITED (INSN_VAR_LOCATION_DECL (insn)) = 0;
}
else if (INSN_P (insn))
{
- count_reg_usage (insn, counts, NULL_RTX, 1);
- note_stores (insn, count_stores, counts + nreg * 2);
+ count_reg_usage (insn, counts, NULL_RTX, 1, nreg);
+ note_stores (insn, count_stores, counts + nreg * 3);
}
- /* If there can be debug insns, COUNTS are 3 consecutive arrays.
+ /* If there can be debug insns, COUNTS are 4 consecutive arrays.
First one counts how many times each pseudo is used outside
of debug insns, second counts how many times each pseudo is
- used in debug insns and third counts how many times a pseudo
- is stored. */
+ used in insns which set that pseudo, third counts how many times
+ each pseudo is used in debug insns and fourth counts how many
+ times a pseudo is stored. */
}
else
{
- counts = XCNEWVEC (int, nreg);
+ counts = XCNEWVEC (int, nreg * 2);
for (insn = insns; insn; insn = NEXT_INSN (insn))
if (INSN_P (insn))
- count_reg_usage (insn, counts, NULL_RTX, 1);
- /* If no debug insns can be present, COUNTS is just an array
- which counts how many times each pseudo is used. */
+ count_reg_usage (insn, counts, NULL_RTX, 1, nreg);
+ /* If no debug insns can be present, COUNTS are just 2 consecutive
+ arrays. First one counts how many times each pseudo is used and
+ second counts how many times each pseudo is used in insns which
+ set that pseudo. */
}
/* Pseudo PIC register should be considered as used due to possible
new usages generated. */
@@ -7085,8 +7126,8 @@ delete_trivially_dead_insns (rtx_insn *i
if (DEBUG_INSN_P (insn))
{
if (DEBUG_BIND_INSN_P (insn))
- count_reg_usage (INSN_VAR_LOCATION_LOC (insn), counts + nreg,
- NULL_RTX, -1);
+ count_reg_usage (INSN_VAR_LOCATION_LOC (insn),
+ counts + nreg * 2, NULL_RTX, -1, 0);
}
else
{
@@ -7095,9 +7136,9 @@ delete_trivially_dead_insns (rtx_insn *i
&& (set = single_set (insn)) != NULL_RTX
&& is_dead_reg (SET_DEST (set), counts)
/* Used at least once in some DEBUG_INSN. */
- && counts[REGNO (SET_DEST (set)) + nreg] > 0
+ && counts[REGNO (SET_DEST (set)) + nreg * 2] > 0
/* And set exactly once. */
- && counts[REGNO (SET_DEST (set)) + nreg * 2] == 1
+ && counts[REGNO (SET_DEST (set)) + nreg * 3] == 1
&& !side_effects_p (SET_SRC (set))
&& asm_noperands (PATTERN (insn)) < 0)
{
@@ -7114,7 +7155,8 @@ delete_trivially_dead_insns (rtx_insn *i
DEBUG_EXPR_TREE_DECL (dval),
SET_SRC (set),
VAR_INIT_STATUS_INITIALIZED);
- count_reg_usage (bind_var_loc, counts + nreg, NULL_RTX, 1);
+ count_reg_usage (bind_var_loc, counts + nreg * 2,
+ NULL_RTX, 1, 0);
bind = emit_debug_insn_before (bind_var_loc, insn);
df_insn_rescan (bind);
@@ -7124,7 +7166,7 @@ delete_trivially_dead_insns (rtx_insn *i
replacements[REGNO (SET_DEST (set))] = dval;
}
- count_reg_usage (insn, counts, NULL_RTX, -1);
+ count_reg_usage (insn, counts, NULL_RTX, -1, nreg);
ndead++;
}
cse_cfg_altered |= delete_insn_and_edges (insn);
@@ -7145,6 +7187,61 @@ delete_trivially_dead_insns (rtx_insn *i
}
}
}
+ if (ndead)
+ for (int i = FIRST_PSEUDO_REGISTER; i < nreg; ++i)
+ if (counts[i] == 0 && counts[i + nreg] != 0)
+ {
+ /* Found at least one pseudo register which had all uses
+ optimized away but at least one insn setting it and
+ using that pseudo inside of its pattern kept. Those
+ need to be removed now. */
+ for (insn = get_last_insn (); insn; insn = prev)
+ {
+ prev = PREV_INSN (insn);
+ if (!NONDEBUG_INSN_P (insn))
+ continue;
+
+ if (GET_CODE (PATTERN (insn)) == SET)
+ {
+ rtx dest = SET_DEST (PATTERN (insn));
+ if (!REG_P (dest)
+ || REGNO (dest) < FIRST_PSEUDO_REGISTER
+ || counts[REGNO (dest)]
+ || !counts[REGNO (dest) + nreg])
+ continue;
+ }
+ else if (GET_CODE (PATTERN (insn)) == PARALLEL)
+ {
+ for (i = XVECLEN (PATTERN (insn), 0) - 1; i >= 0; i--)
+ {
+ rtx elt = XVECEXP (PATTERN (insn), 0, i);
+
+ if (GET_CODE (elt) == SET)
+ {
+ if (set_noop_p (elt))
+ continue;
+ if (!REG_P (SET_DEST (elt))
+ || REGNO (SET_DEST (elt)) < FIRST_PSEUDO_REGISTER
+ || counts[REGNO (SET_DEST (elt))]
+ || !counts[REGNO (SET_DEST (elt)) + nreg])
+ break;
+ }
+ else if (GET_CODE (elt) != CLOBBER
+ && GET_CODE (elt) != USE)
+ break;
+ }
+ if (i >= 0)
+ continue;
+ }
+ else
+ continue;
+
+ gcc_checking_assert (!insn_live_p (insn, counts));
+ ndead++;
+ cse_cfg_altered |= delete_insn_and_edges (insn);
+ }
+ break;
+ }
if (MAY_HAVE_DEBUG_BIND_INSNS)
{
--- 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
