tree, c++: Consider TARGET_EXPR invariant like SAVE_EXPR [PR118509]

My October PR117259 fix to get_member_function_from_ptrfunc to use a
TARGET_EXPR rather than SAVE_EXPR unfortunately caused some regressions as
well as the following testcase shows.
What happens is that
get_member_function_from_ptrfunc -> build_base_path calls save_expr,
so since the PR117259 change in mnay cases it will call save_expr on
a TARGET_EXPR.  And, for some strange reason a TARGET_EXPR is not considered
an invariant, so we get a SAVE_EXPR wrapped around the TARGET_EXPR.
That SAVE_EXPR <TARGET_EXPR <...>> gets initially added only to the second
operand of ?:, so at that point it would still work fine during expansion.
But unfortunately an expression with that subexpression is handed to the
caller also through *instance_ptrptr = instance_ptr; and gets evaluated
once again when computing the first argument to the method.
So, essentially, we end up with
(TARGET_EXPR <D.2907, ...>, (... ? ... SAVE_EXPR <TARGET_EXPR <D.2907, ...>
 ... : ...)) (... SAVE_EXPR <TARGET_EXPR <D.2907, ...> ..., ...);
and while D.2907 is initialized during gimplification in the code dominating
everything that uses it, the extra temporary created for the SAVE_EXPR
is initialized only conditionally (if the ?: condition is true) but then
used unconditionally, so we get
pmf-4.C: In function ‘void foo(C, B*)’:
pmf-4.C:12:11: warning: ‘<anonymous>’ may be used uninitialized [-Wmaybe-uninitialized]
   12 |   (y->*x) ();
      |   ~~~~~~~~^~
pmf-4.C:12:11: note: ‘<anonymous>’ was declared here
   12 |   (y->*x) ();
      |   ~~~~~~~~^~
diagnostic and wrong-code issue too.

The following patch fixes it by considering a TARGET_EXPR invariant
for SAVE_EXPR purposes the same as SAVE_EXPR is.  Really creating another
temporary for it is just a waste of the IL.

Unfortunately I had to tweak the omp matching code to be able to accept
TARGET_EXPR the same as SAVE_EXPR.

2025-01-20  Jakub Jelinek  <jakub@redhat.com>

	PR c++/118509
gcc/
	* tree.cc (tree_invariant_p_1): Return true for TARGET_EXPR too.
gcc/c-family/
	* c-omp.cc (c_finish_omp_for): Handle TARGET_EXPR in first operand
	of COMPOUND_EXPR incr the same as SAVE_EXPR.
gcc/testsuite/
	* g++.dg/expr/pmf-4.C: New test.

diff --git a/gcc/c-family/c-omp.cc b/gcc/c-family/c-omp.cc
index a65b1492eca6c406db4613cc5c87704bb1bf067f..a92c6e3dc3163149f2a80d400d3c66e0f756cb31 100644 (file)
--- a/gcc/c-family/c-omp.cc
+++ b/gcc/c-family/c-omp.cc
@@ -1195,7 +1195,8 @@ c_finish_omp_for (location_t locus, enum tree_code code, tree declv,
              break;
 
            case COMPOUND_EXPR:
-             if (TREE_CODE (TREE_OPERAND (incr, 0)) != SAVE_EXPR
+             if ((TREE_CODE (TREE_OPERAND (incr, 0)) != SAVE_EXPR
+                  && TREE_CODE (TREE_OPERAND (incr, 0)) != TARGET_EXPR)
                  || TREE_CODE (TREE_OPERAND (incr, 1)) != MODIFY_EXPR)
                break;
              incr = TREE_OPERAND (incr, 1);

diff --git a/gcc/testsuite/g++.dg/expr/pmf-4.C b/gcc/testsuite/g++.dg/expr/pmf-4.C
new file mode 100644 (file)
index 0000000..87c9be1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/pmf-4.C
@@ -0,0 +1,22 @@
+// PR c++/118509
+// { dg-do run }
+// { dg-options "-Wall -O2" }
+
+struct A { void foo () { a = 1; } int a; A () : a (0) {} };
+struct B : virtual A {};
+typedef void (A::*C) ();
+
+__attribute__((noipa)) void
+foo (C x, B *y)
+{
+  (y->*x) ();
+}
+
+int
+main ()
+{
+  B b;
+  foo (&A::foo, &b);
+  if (b.a != 1)
+    __builtin_abort ();
+}

diff --git a/gcc/tree.cc b/gcc/tree.cc
index eab40008e8baf6fde1accba24487f248f746c79a..05f679edc091b7693c563b1c8d1277cf88bcfef4 100644 (file)
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -3922,6 +3922,7 @@ tree_invariant_p_1 (tree t)
   switch (TREE_CODE (t))
     {
     case SAVE_EXPR:
+    case TARGET_EXPR:
       return true;
 
     case ADDR_EXPR: