On 1/16/25 8:04 PM, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?-- >8 -- The recent r15-6369 unfortunately caused a bad wrong-code issue. Here we have TARGET_EXPR <D.2996, (void) (D.2996 = {.status=0, .data={._vptr.Foo=&_ZTV3Foo + 16}})> and call cp_fold_r -> maybe_constant_init with object=D.2996. In cxx_eval_outermost_constant_expr we now take the type of the object if present. An object can't have type 'void' and so we continue to evaluate the initializer. That evaluates into a VOID_CST, meaning we disregard the whole initializer, and terrible things ensue.
In that case, I'd think we want to use the value of 'object' (which should be in ctx.ctor?) instead of the return value of cxx_eval_constant_expression.
I think the new prvalue optimization (r15-6052) should only be enabled for simple TARGET_EXPRs -- ones that don't initialize the object via a sequence of statement as the one above. I also think we want an assert so that this doesn't happen again. PR c++/118396 PR c++/118523 gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_outermost_constant_expr): Assert that we don't throw away the initializer by evaluating it away. * cp-gimplify.cc (cp_fold_r): Only attempt to evaluate the initializer if the TARGET_EXPR is simple. gcc/testsuite/ChangeLog: * g++.dg/tree-ssa/pr78687.C: Revert r15-6052. * g++.dg/tree-ssa/pr90883.C: Likewise. * g++.dg/cpp0x/constexpr-prvalue4.C: New test. * g++.dg/cpp1y/constexpr-prvalue3.C: New test. --- gcc/cp/constexpr.cc | 14 ++++-- gcc/cp/cp-gimplify.cc | 4 +- .../g++.dg/cpp0x/constexpr-prvalue4.C | 33 ++++++++++++++ .../g++.dg/cpp1y/constexpr-prvalue3.C | 45 +++++++++++++++++++ gcc/testsuite/g++.dg/tree-ssa/pr78687.C | 3 +- gcc/testsuite/g++.dg/tree-ssa/pr90883.C | 3 +- 6 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index c898e3bfa6e..61b25277419 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -8876,9 +8876,17 @@ cxx_eval_outermost_constant_expr (tree t, bool allow_non_constant, /* Turn off -frounding-math for manifestly constant evaluation. */ warning_sentinel rm (flag_rounding_math, ctx.manifestly_const_eval == mce_true); - tree type = (object - ? cv_unqualified (TREE_TYPE (object)) - : initialized_type (t)); + tree type; + if (object) + { + type = cv_unqualified (TREE_TYPE (object)); + /* If there is an object to initialize, make sure we don't throw + away the initializer. */ + gcc_assert (!VOID_TYPE_P (initialized_type (t)) || constexpr_dtor); + } + else + type = initialized_type (t); + tree r = t; bool is_consteval = false; if (VOID_TYPE_P (type)) diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc index c7074b00cef..283c0fa3e26 100644 --- a/gcc/cp/cp-gimplify.cc +++ b/gcc/cp/cp-gimplify.cc @@ -1475,7 +1475,9 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_) cp_walk_tree (&init, cp_fold_r, data, NULL); cp_walk_tree (&TARGET_EXPR_CLEANUP (stmt), cp_fold_r, data, NULL); *walk_subtrees = 0; - if (!flag_no_inline) + /* Only attempt to evaluate the initializer if we're inlining and + the TARGET_EXPR is simple. */ + if (!flag_no_inline && !VOID_TYPE_P (TREE_TYPE (init))) { tree folded = maybe_constant_init (init, TARGET_EXPR_SLOT (stmt)); if (folded != init && TREE_CONSTANT (folded)) diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C new file mode 100644 index 00000000000..afcee65f880 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C @@ -0,0 +1,33 @@ +// PR c++/118396 +// { dg-do run { target c++11 } } +// { dg-options "-O" } + +void *operator new(__SIZE_TYPE__, void *__p) { return __p; } + +struct Foo { + virtual ~Foo() = default; +}; +struct Data { + int status; + Foo data{}; +}; + +Data *P, *Q; + +struct vector { + vector (const Data &__value) { + P = static_cast<Data *>(__builtin_operator_new(0)); + new (P) Data (__value); + Q = P + 1; + } + Data *begin() { return P; } + Data *end() { return Q; } +}; + +int +main () +{ + vector items_(Data{}); + for (auto item : items_) + item.status == 0 ? void() : __builtin_abort (); +} diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C new file mode 100644 index 00000000000..8ea86c60be5 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C @@ -0,0 +1,45 @@ +// PR c++/118523 +// { dg-do compile { target c++14 } } +// { dg-options "-O2 -Wall" } + +struct __new_allocator { + constexpr __new_allocator() {} + __new_allocator(__new_allocator &) {} +}; +template <typename> using __allocator_base = __new_allocator; +template <typename> struct allocator_traits; +template <typename> struct allocator : __allocator_base<int> {}; +template <typename _Tp> struct allocator_traits<allocator<_Tp>> { + using pointer = _Tp *; + template <typename _Up> using rebind_alloc = allocator<_Up>; + static void deallocate(allocator<_Tp>, pointer, long); +}; +struct __alloc_traits : allocator_traits<allocator<int>> {}; +struct _Vector_impl_data { + __alloc_traits::pointer _M_start; + __alloc_traits::pointer _M_end_of_storage; + constexpr _Vector_impl_data() : _M_start(), _M_end_of_storage() {} +}; +struct _Vector_impl : __alloc_traits::rebind_alloc<int>, _Vector_impl_data {}; +struct _Vector_base { + ~_Vector_base() { + _M_deallocate(_M_impl._M_start, + _M_impl._M_end_of_storage - _M_impl._M_start); + } + _Vector_impl _M_impl; + void _M_deallocate(__alloc_traits::pointer __p, long __n) { + if (__p) + __alloc_traits::deallocate(_M_impl, __p, __n); + } +}; +struct vector : protected _Vector_base {}; +struct S { + vector a{}; +}; +struct B2 { + B2(S); +}; +struct E : B2 { + E(S opts = {}) : B2{opts} {} +}; +void fun() { E{}; } diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr78687.C b/gcc/testsuite/g++.dg/tree-ssa/pr78687.C index 6e8a5b7efdf..698458f0e9a 100644 --- a/gcc/testsuite/g++.dg/tree-ssa/pr78687.C +++ b/gcc/testsuite/g++.dg/tree-ssa/pr78687.C @@ -480,5 +480,4 @@ int main(int argc, char* argv[]) f(); }-// XFAILed due to PR116416-/* { dg-final { scan-tree-dump "Removing load:.*ptr;" "sra" { xfail { *-*-* } } } } */ +/* { dg-final { scan-tree-dump "Removing load:.*ptr;" "sra" } } */ diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr90883.C b/gcc/testsuite/g++.dg/tree-ssa/pr90883.C index ad9231eaff2..f540e78fa00 100644 --- a/gcc/testsuite/g++.dg/tree-ssa/pr90883.C +++ b/gcc/testsuite/g++.dg/tree-ssa/pr90883.C @@ -16,5 +16,4 @@ // We want to match enough here to capture that we deleted an empty // constructor store // mips will expand to loop to clear because CLEAR_RATIO. -// { dg-final { scan-tree-dump-not ".*\.a = {}" "dse1" { xfail { mips*-*-* } } } } -// { dg-final { scan-tree-dump-not ".*\.b = 0" "dse1" { xfail { mips*-*-* } } } } +// { dg-final { scan-tree-dump "Deleted redundant store: .*\.a = {}" "dse1" { xfail { mips*-*-* } } } } base-commit: 29da6a642402ac64002f5edeab268606b4637103
