Re: [4.9] PR 62146

[Easwaran Raman <eraman at google dot com>]

I've attached the revised patch. Bootstrapped and no test regressions
on x86_64/linux with 4.9 branch. Ok for 4.9 branch? While the bug
doesn't show up in trunk, seems obvious that this should go to trunk
as well. Is it ok for trunk if tests pass?

Btw, is g++.dg/opt the right directory for the test to go?

-----

ChangeLog

2014-09-04  Easwaran Raman  <eraman@google.com>

        PR rtl-optimization/62146
        * ifcvt.c (dead_or_predicable): Make removal of REG_EQUAL note of
        hoisted instruction unconditional.

2014-09-04  Easwaran Raman  <eraman@google.com>

        PR rtl-optimization/62146
        * testsuite/g++.dg/opt/pr62146.C: New.


On Wed, Sep 3, 2014 at 11:36 AM, Jeff Law <law@redhat.com> wrote:
> On 09/02/14 12:52, Easwaran Raman wrote:
>>
>> It turns out that the REG_EQUAL note is removed on a hoisted
>> instruction (relevant code is in dead_or_predicable in ifcvt.c) if the
>> source of the move instruction is not a function invariant. In this
>> case, the source is a function invariant (constant) and so that
>> doesn't kick in. I don't understand why this exemption for function
>> invariant is there and the original thread in
>> https://gcc.gnu.org/ml/gcc/2005-05/msg01710.html doesn't explain
>> either. Should I just remove the REG_EQUAL notes of all hoisted
>> instructions or are there cases where it is safe to leave the note?
>
> I suspect it was left in in an attempt to keep as many REG_EQUAL notes as
> possible as the notes can help later optimizations.  But even those
> equivalences are not necessarily safe.
>
> I'm pretty sure the right thing to do here is just drop the note regardless
> of whether or not its an invariant.
>
> jeff
>

Index: testsuite/g++.dg/opt/pr62146.C
===================================================================
--- testsuite/g++.dg/opt/pr62146.C	(revision 0)
+++ testsuite/g++.dg/opt/pr62146.C	(revision 0)
@@ -0,0 +1,51 @@
+/* PR rtl-optimization/62146 */
+/* { dg-do compile } */
+/* { dg-options "-O2 " } */
+class F
+{
+public:
+    virtual ~ F ();
+};
+template < class CL > class G:public F
+{
+    int *member_;
+public:
+    G ( int *b): member_ (0)
+    {
+    }
+};
+
+class D
+{
+public:
+    template < class CL > void RegisterNonTagCallback (int,
+            void (CL::
+                  *p3) ())
+    {
+        InternalRegisterNonTag (p3 ? new G < CL > ( 0) : 0);
+    } void InternalRegisterNonTag (F *);
+};
+
+void fn1 ();
+class C1
+{
+    void  foo();
+    class TokenType
+    {
+    public:
+        void AddToken ()
+        {
+        }
+    };
+    C1::TokenType bar_t;
+};
+D a;
+void C1::foo()
+{
+    if (&bar_t)
+        fn1 ();
+    for (int i = 0; i < sizeof 0; ++i)
+        a.RegisterNonTagCallback (0, &TokenType::AddToken);
+}
+
+/* { dg-final { scan-assembler-not "mov.*_ZN2C19TokenType8AddTokenEv, .\\\(" } } */
Index: ifcvt.c
===================================================================
--- ifcvt.c	(revision 214472)
+++ ifcvt.c	(working copy)
@@ -4387,17 +4387,14 @@ dead_or_predicable (basic_block test_bb, basic_blo
       insn = head;
       do
 	{
-	  rtx note, set;
+	  rtx note;
 
 	  if (! INSN_P (insn))
 	    continue;
 	  note = find_reg_note (insn, REG_EQUAL, NULL_RTX);
 	  if (! note)
 	    continue;
-	  set = single_set (insn);
-	  if (!set || !function_invariant_p (SET_SRC (set))
-	      || !function_invariant_p (XEXP (note, 0)))
-	    remove_note (insn, note);
+	  remove_note (insn, note);
 	} while (insn != end && (insn = NEXT_INSN (insn)));
 
       /* PR46315: when moving insns above a conditional branch, the REG_EQUAL