diff options
author | Jakub Jelinek <jakub@redhat.com> | 2025-01-20 10:26:49 +0100 |
---|---|---|
committer | Jakub Jelinek <jakub@gcc.gnu.org> | 2025-01-20 10:26:49 +0100 |
commit | d9d0eeea93d39d304c7420e87f4b903d89f2e9fa (patch) | |
tree | d827dea58c7a161192befb03aaea997db8b8e854 /gcc | |
parent | d882e48d48bf300941c3610c5af157c64ccf0a84 (diff) | |
download | gcc-d9d0eeea93d39d304c7420e87f4b903d89f2e9fa.zip gcc-d9d0eeea93d39d304c7420e87f4b903d89f2e9fa.tar.gz gcc-d9d0eeea93d39d304c7420e87f4b903d89f2e9fa.tar.bz2 |
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.
Diffstat (limited to 'gcc')
-rw-r--r-- | gcc/c-family/c-omp.cc | 3 | ||||
-rw-r--r-- | gcc/testsuite/g++.dg/expr/pmf-4.C | 22 | ||||
-rw-r--r-- | gcc/tree.cc | 1 |
3 files changed, 25 insertions, 1 deletions
diff --git a/gcc/c-family/c-omp.cc b/gcc/c-family/c-omp.cc index a65b149..a92c6e3 100644 --- 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 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 eab4000..05f679e 100644 --- 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: |