https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117191
--- Comment #6 from denisc at gcc dot gnu.org ---
The patch:
(In reply to denisc from comment #4)
> (In reply to denisc from comment #0)
> > Created attachment 59370 [details]
> > dse2 pass dump file
> >
> > Failed testcase:
> > $ make -k check-gcc RUNTESTFLAGS="--target_board=atmega128-sim
> > --tool_opts='-mlra' avr-torture.exp=lra-pr116550-2.c"
> > [...]
> > Running
> > /mnt/d/gcc-avr-lra/gcc/testsuite/gcc.target/avr/torture/avr-torture.exp ...
> > FAIL: gcc.target/avr/torture/lra-pr116550-2.c -O1 execution test
> >
> > === gcc Summary ===
> >
> > # of expected passes 21
> > # of unexpected failures 1
> > /mnt/d/mk-avr-lra/gcc/xgcc version 15.0.0 20241016 (experimental) (GCC)
> > [...]
> >
> >
> > This test successfully passed without -mlra option, may be lra related:
> > $ make -k check-gcc RUNTESTFLAGS="--target_board=atmega128-sim
> > --tool_opts='-mno-lra' avr-torture.exp=lra-pr116550-2.c"
> > [...]
> > Running
> > /mnt/d/gcc-avr-lra/gcc/testsuite/gcc.target/avr/torture/avr-torture.exp ...
> >
> > === gcc Summary ===
> >
> > # of expected passes 22
> > /mnt/d/mk-avr-lra/gcc/xgcc version 15.0.0 20241016 (experimental) (GCC)
> > [...]
> >
> > Testcase
> >
> > $ avr-gcc -da -O1 -S -dp -fverbose-asm -mmcu=atmega128 lra-pr116550-2.c
> >
> >
> > dse2 pass made a wrong elimination of insn 554 (%sfp+13)
> > insn 456 have a use of *(%sfp+13):QI
> > probably scanning of it have to be like:
> > **scanning insn=505
> > mem: (plus:HI (reg/f:HI 28 r28)
> > (const_int 5 [0x5]))
> >
> > after canon_rtx address: (plus:HI (reg/f:HI 28 r28)
> > (const_int 5 [0x5]))
> > gid=1 offset=5
> > processing const load gid=1[5..6)
> > removing from active insn=459 has store
> > mems_found = 0, cannot_delete = true
> >
> > but it's
> > **scanning insn=456
> > mem: (plus:HI (reg/f:HI 28 r28)
> > (const_int 13 [0xd]))
> >
> > after canon_rtx address: (plus:HI (reg/f:HI 28 r28)
> > (const_int 13 [0xd]))
> > gid=1 offset=13
> > processing const load gid=1[13..14)
> > mems_found = 0, cannot_delete = true
> >
> > Scanning of insn 456 doesn't have 'removing from active insn=554 has store'
> > because of that insn 554 was removed but it have a store to *(%sfp+13)
> > It's wrong because insn 456 have a use of *(%sfp+13)
diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index c149c3388cd..db591e1d12c 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -741,10 +741,19 @@ lra_final_code_change (void)
delete_insn (insn);
continue;
}
- if (GET_CODE (pat) == CLOBBER && LRA_TEMP_CLOBBER_P (pat))
+ if (GET_CODE (pat) == CLOBBER
+ && (LRA_TEMP_CLOBBER_P (pat)
+ || (!MEM_P (XEXP (pat, 0))
+ || GET_MODE (XEXP (pat, 0)) != BLKmode
+ || (GET_CODE (XEXP (XEXP (pat, 0), 0)) != SCRATCH
+ && XEXP (XEXP (pat, 0), 0)
+ != stack_pointer_rtx))))
{
- /* Remove clobbers temporarily created in LRA. We don't
- need them anymore and don't want to waste compiler
+ /* Remove clobbers temporarily created in LRA and clobbers with
+ registers spilled to memory except those that refer to the
+ return value and the special mem:BLK CLOBBERs added to prevent
+ the scheduler from misarranging variable-array code.
+ We don't need them anymore and don't want to waste compiler
time processing them in a few subsequent passes. */
diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index c149c3388cd..db591e1d12c 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -741,10 +741,19 @@ lra_final_code_change (void)
delete_insn (insn);
continue;
}
- if (GET_CODE (pat) == CLOBBER && LRA_TEMP_CLOBBER_P (pat))
+ if (GET_CODE (pat) == CLOBBER
+ && (LRA_TEMP_CLOBBER_P (pat)
+ || (!MEM_P (XEXP (pat, 0))
+ || GET_MODE (XEXP (pat, 0)) != BLKmode
+ || (GET_CODE (XEXP (XEXP (pat, 0), 0)) != SCRATCH
+ && XEXP (XEXP (pat, 0), 0)
+ != stack_pointer_rtx))))
{
- /* Remove clobbers temporarily created in LRA. We don't
- need them anymore and don't want to waste compiler
+ /* Remove clobbers temporarily created in LRA and clobbers with
+ registers spilled to memory except those that refer to the
+ return value and the special mem:BLK CLOBBERs added to prevent
+ the scheduler from misarranging variable-array code.
+ We don't need them anymore and don't want to waste compiler
time processing them in a few subsequent passes. */
lra_invalidate_insn_data (insn);
delete_insn (insn);
lra_invalidate_insn_data (insn);
delete_insn (insn);
>
> These conclusions were wrong because it's a local (inter basic blocks)
> algorythms.
>
> We have a problem in global DSE.
Probably I found a bug.
The bug appears after the dse2 pass.
The dse2 pass removes necessary insns. (ie insn 554)
They are removed because LRA doesn't remove CLOBBER insn. (insn 202)
The old "good" reload has a special "pass" for removing such CLOBBER insns.
Fragment from reload1.c:
-------------------------------------------------------------------------------
reload_completed = 1;
/* Make a pass over all the insns and delete all USEs which we inserted
only to tag a REG_EQUAL note on them. Remove all REG_DEAD and REG_UNUSED
notes. Delete all CLOBBER insns, except those that refer to the return
value and the special mem:BLK CLOBBERs added to prevent the scheduler
from misarranging variable-array code, and simplify (subreg (reg))
operands. Strip and regenerate REG_INC notes that may have been moved
around. */
for (insn = first; insn; insn = NEXT_INSN (insn))
if (INSN_P (insn))
{
rtx *pnote;
if (CALL_P (insn))
replace_pseudos_in (& CALL_INSN_FUNCTION_USAGE (insn),
VOIDmode, CALL_INSN_FUNCTION_USAGE (insn));
if ((GET_CODE (PATTERN (insn)) == USE
/* We mark with QImode USEs introduced by reload itself. */
&& (GET_MODE (insn) == QImode
|| find_reg_note (insn, REG_EQUAL, NULL_RTX)))
|| (GET_CODE (PATTERN (insn)) == CLOBBER
&& (!MEM_P (XEXP (PATTERN (insn), 0))
|| GET_MODE (XEXP (PATTERN (insn), 0)) != BLKmode
|| (GET_CODE (XEXP (XEXP (PATTERN (insn), 0), 0)) !=
SCRATCH
&& XEXP (XEXP (PATTERN (insn), 0), 0)
!= stack_pointer_rtx))
&& (!REG_P (XEXP (PATTERN (insn), 0))
|| ! REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0)))))
{
delete_insn (insn);
continue;
}
-------------------------------------------------------------------------------
The fix is simple:
diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index c149c3388cd..db591e1d12c 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -741,10 +741,19 @@ lra_final_code_change (void)
delete_insn (insn);
continue;
}
- if (GET_CODE (pat) == CLOBBER && LRA_TEMP_CLOBBER_P (pat))
+ if (GET_CODE (pat) == CLOBBER
+ && (LRA_TEMP_CLOBBER_P (pat)
+ || (!MEM_P (XEXP (pat, 0))
+ || GET_MODE (XEXP (pat, 0)) != BLKmode
+ || (GET_CODE (XEXP (XEXP (pat, 0), 0)) != SCRATCH
+ && XEXP (XEXP (pat, 0), 0)
+ != stack_pointer_rtx))))
{
- /* Remove clobbers temporarily created in LRA. We don't
- need them anymore and don't want to waste compiler
+ /* Remove clobbers temporarily created in LRA and clobbers with
+ registers spilled to memory except those that refer to the
+ return value and the special mem:BLK CLOBBERs added to prevent
+ the scheduler from misarranging variable-array code.
+ We don't need them anymore and don't want to waste compiler
time processing them in a few subsequent passes. */
lra_invalidate_insn_data (insn);
delete_insn (insn);
