> Am 02.12.2023 um 14:15 schrieb Richard Sandiford <richard.sandif...@arm.com>:
>
> Eric Botcazou <botca...@adacore.com> writes:
>>> 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.
>>
>> We have doing that for the past 15 years though, so what has changed?
>
> Off-hand, I couldn't remember a case where we'd done this specifically
> for false-positive REG_UNUSED notes. (There probably were cases though.)
>
>>> (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.)
>>
>> Reintroducing the manual management of such notes would be a step backward.
>
> I think false-positive REG_UNUSED notes are fundamentally different
> from the other cases though. If a register is unused then its natural
> state is to remain unused. The register will only become used if something
> goes out of its way to add new uses of an instruction result that "just
> happens to be there". That's a deliberate decision and needs some
> analysis to prove that it's safe. Requiring the pass to clear REG_UNUSED
> notes too doesn't seem like a significant extra burden.
>
> Trying to reduce false-negative REG_UNUSED notes is different,
> since deleting any instruction could in principle make a register
> go from used to unused. Same for REG_DEAD notes: if a pass deletes
> an instruction with a REG_DEAD note then it shouldn't have to figure
> out where the new death occurs.
>
> Not sure how representative this is, but I tried the hack below
> to flag cases where single_set is used in passes that don't have
> up-to-date notes, then ran it on execute.exp. The checking fired
> for every version of every test. The collected passes were:
>
> single_set: bbro
> single_set: cc_fusion
> single_set: ce1
> single_set: ce2
> single_set: ce3
> single_set: cmpelim
> single_set: combine
> single_set: compgotos
> single_set: cprop
> single_set: dwarf2
> single_set: fold_mem_offsets
> single_set: fwprop1
> single_set: fwprop2
> single_set: gcse2
> single_set: hoist
> single_set: init-regs
> single_set: ira
> single_set: jump2
> single_set: jump_after_combine
> single_set: loop2_done
> single_set: loop2_invariant
> single_set: postreload
> single_set: pro_and_epilogue
> single_set: ree
> single_set: reload
> single_set: rtl_dce
> single_set: rtl pre
> single_set: sched1
> single_set: sched2
> single_set: sched_fusion
> single_set: sms
> single_set: split1
> single_set: split2
> single_set: split3
> single_set: split5
> single_set: subreg3
> single_set: ud_dce
> single_set: vartrack
> single_set: web
>
> which is a lot of passes :)
>
> Some of the calls might be OK in context, due to call-specific
> circumstances. But I think it's generally easier to see/show that
> a pass is adding new uses of existing defs than it is to prove that
> a use of single_set is safe even when notes aren't up-to-date.
I think this asks for a verify_notes that checks if notes are conservatively
correct as to our definition then? Not sure if doable for equal/equiv notes
though.
Richard
> Thanks,
> Richard
>
>
> diff --git a/gcc/df-problems.cc b/gcc/df-problems.cc
> index d2cfaf7f50f..ece49f041e0 100644
> --- a/gcc/df-problems.cc
> +++ b/gcc/df-problems.cc
> @@ -3782,6 +3782,8 @@ void
> df_note_add_problem (void)
> {
> df_add_problem (&problem_NOTE);
> + extern bool single_set_ok;
> + single_set_ok = true;
> }
>
>
> diff --git a/gcc/passes.cc b/gcc/passes.cc
> index 6f894a41d22..d8e12ea2512 100644
> --- a/gcc/passes.cc
> +++ b/gcc/passes.cc
> @@ -2637,9 +2637,14 @@ execute_one_pass (opt_pass *pass)
> do_per_function (verify_curr_properties,
> (void *)(size_t)pass->properties_required);
>
> + extern bool single_set_ok;
> + single_set_ok = !df;
> +
> /* Do it! */
> todo_after = pass->execute (cfun);
>
> + single_set_ok = !df;
> +
> if (todo_after & TODO_discard_function)
> {
> /* Stop timevar. */
> diff --git a/gcc/rtl.h b/gcc/rtl.h
> index e4b6cc0dbb5..af3bd1b7cfa 100644
> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -3607,8 +3607,11 @@ extern void add_auto_inc_notes (rtx_insn *, rtx);
>
> /* Handle the cheap and common cases inline for performance. */
>
> +extern void check_single_set ();
> inline rtx single_set (const rtx_insn *insn)
> {
> + check_single_set ();
> +
> if (!INSN_P (insn))
> return NULL_RTX;
>
> diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
> index 87267ee3b88..e0207e2753a 100644
> --- a/gcc/rtlanal.cc
> +++ b/gcc/rtlanal.cc
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. If not see
> #include "rtl-iter.h"
> #include "hard-reg-set.h"
> #include "function-abi.h"
> +#include "tree-pass.h"
>
> /* Forward declarations */
> static void set_of_1 (rtx, const_rtx, void *);
> @@ -1543,6 +1544,20 @@ record_hard_reg_uses (rtx *px, void *data)
> It may also have CLOBBERs, USEs, or SET whose output
> will not be used, which we ignore. */
>
> +bool single_set_ok;
> +void
> +check_single_set ()
> +{
> + static opt_pass *last_pass;
> + if (!single_set_ok
> + && current_pass
> + && last_pass != current_pass)
> + {
> + last_pass = current_pass;
> + fprintf (stderr, "single_set: %s\n", current_pass->name);
> + }
> +}
> +
> rtx
> single_set_2 (const rtx_insn *insn, const_rtx pat)
> {
