Jakub Jelinek <ja...@redhat.com> writes:
> Hi!
>
> The following testcase ICEs on x86_64-linux since df_note_add_problem ()
> call has been added to mode switching.
> The problem is that the pro_and_epilogue pass in
> prepare_shrink_wrap -> copyprop_hardreg_forward_bb_without_debug_insn
> uses regcprop.cc infrastructure which relies on accurate REG_DEAD/REG_UNUSED
> notes. E.g. regcprop.cc
> /* We need accurate notes. Earlier passes such as if-conversion may
> leave notes in an inconsistent state. */
> df_note_add_problem ();
> documents that. On the testcase below it is in particular the
> /* Detect obviously dead sets (via REG_UNUSED notes) and remove them.
> */
> if (set
> && !RTX_FRAME_RELATED_P (insn)
> && NONJUMP_INSN_P (insn)
> && !may_trap_p (set)
> && find_reg_note (insn, REG_UNUSED, SET_DEST (set))
> && !side_effects_p (SET_SRC (set))
> && !side_effects_p (SET_DEST (set)))
> {
> bool last = insn == BB_END (bb);
> delete_insn (insn);
> if (last)
> break;
> continue;
> }
> case where a stale REG_UNUSED note breaks stuff up (added in vzeroupper
> pass, redundant insn after it deleted later).
>
> The following patch makes sure the notes are not stale if shrink wrapping
> is enabled.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I still maintain that so much stuff relies on the lack of false-positive
REG_UNUSED notes that (whatever the intention might have been) we need
to prevent the false positive. Like Andrew says, any use of single_set
is suspect if there's a REG_UNUSED note for something that is in fact used.
So sorry to be awkward, but I don't think this is the way to go. I think
we'll just end up playing whack-a-mole and adding df_note_add_problem to
lots of passes.
(FTR, I'm not saying passes have to avoid false negatives, just false
positives. If a pass updates an instruction with a REG_UNUSED note,
and the pass is no longer sure whether the register is unused or not,
the pass can just delete the note.)
Richard
> 2023-12-02 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/112760
> * function.cc (thread_prologue_and_epilogue_insns): If
> SHRINK_WRAPPING_ENABLED, call df_note_add_problem before calling
> df_analyze.
>
> * gcc.dg/pr112760.c: New test.
>
> --- gcc/function.cc.jj 2023-11-07 08:32:01.699254744 +0100
> +++ gcc/function.cc 2023-12-01 13:42:51.885189341 +0100
> @@ -6036,6 +6036,11 @@ make_epilogue_seq (void)
> void
> thread_prologue_and_epilogue_insns (void)
> {
> + /* prepare_shrink_wrap uses copyprop_hardreg_forward_bb_without_debug_insn
> + which uses regcprop.cc functions which rely on accurate REG_UNUSED
> + and REG_DEAD notes. */
> + if (SHRINK_WRAPPING_ENABLED)
> + df_note_add_problem ();
> df_analyze ();
>
> /* Can't deal with multiple successors of the entry block at the
> --- gcc/testsuite/gcc.dg/pr112760.c.jj 2023-12-01 13:46:57.444746529
> +0100
> +++ gcc/testsuite/gcc.dg/pr112760.c 2023-12-01 13:46:36.729036971 +0100
> @@ -0,0 +1,22 @@
> +/* PR rtl-optimization/112760 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fno-dce -fno-guess-branch-probability
> --param=max-cse-insns=0" } */
> +/* { dg-additional-options "-m8bit-idiv -mavx" { target i?86-*-* x86_64-*-*
> } } */
> +
> +unsigned g;
> +
> +__attribute__((__noipa__)) unsigned short
> +foo (unsigned short a, unsigned short b)
> +{
> + unsigned short x = __builtin_add_overflow_p (a, g, (unsigned short) 0);
> + g -= g / b;
> + return x;
> +}
> +
> +int
> +main ()
> +{
> + unsigned short x = foo (40, 6);
> + if (x != 0)
> + __builtin_abort ();
> +}
>
> Jakub
