Re: [PATCH] pro_and_epilogue: Call df_note_add_problem () if SHRINK_WRAPPING_ENABLED [PR112760]

[Richard Sandiford <richard.sandiford@arm.com>]

Eric Botcazou <botcazou@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.

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)
 {