| Message ID | CAFk2RUYNJnQRm6w=pf7FohqWn8wiGWVgoJ8RdkE3M-yCs9aGbQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Remove the null check from placement new in all modes | expand |
OK. On Fri, Nov 10, 2017 at 12:29 AM, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > Tested manually on Linux-X64, finishing testing with the full suite on > Linux-PPC64. > > I spent far too much time contemplating whether to add a compatibility switch > for this, but -fcheck-new *is* such a compatibility switch. > > OK for trunk? > > 2017-11-10 Ville Voutilainen <ville.voutilainen@gmail.com> > > gcc/ > > Remove the null check from placement new in all modes > * cp/init.c (build_new_1): Don't do a null check for > a namespace-scope non-replaceable placement new > in any mode unless -fcheck-new is provided. > > testsuite/ > > Remove the null check from placement new in all modes > * g++.dg/init/pr35878_1.C: Adjust. > * g++.dg/init/pr35878_4.C: New.
On 10 November 2017 at 18:19, Jason Merrill <jason@redhat.com> wrote:
> OK.
I see non-obvious testsuite failures, I'll see if adding -fcheck-new
to those cases
cures the problem. Please stay tuned. :)
On 10 November 2017 at 18:55, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > On 10 November 2017 at 18:19, Jason Merrill <jason@redhat.com> wrote: >> OK. > > > I see non-obvious testsuite failures, I'll see if adding -fcheck-new > to those cases > cures the problem. Please stay tuned. :) Here. Tested on Linux-PPC64 and Linux-x64. 2017-11-13 Ville Voutilainen <ville.voutilainen@gmail.com> gcc/ Remove the null check from placement new in all modes * cp/init.c (build_new_1): Don't do a null check for a namespace-scope non-replaceable placement new in any mode unless -fcheck-new is provided. testsuite/ Remove the null check from placement new in all modes * g++.dg/init/pr35878_1.C: Adjust. * g++.dg/init/pr35878_4.C: New. * g++.dg/torture/pr48695.C: Adjust. * g++.dg/tree-ssa/pr31146-2.C: Likewise. * g++.dg/tree-ssa/pr31146-3.C: New. * g++.dg/tree-ssa/pr41428-2.C: Likewise. * g++.dg/tree-ssa/pr41428.C: Adjust. diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 9e6e3af..1fcd91d 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -2758,7 +2758,7 @@ malloc_alignment () static bool std_placement_new_fn_p (tree alloc_fn) { - if ((cxx_dialect > cxx14) && DECL_NAMESPACE_SCOPE_P (alloc_fn)) + if (DECL_NAMESPACE_SCOPE_P (alloc_fn)) { tree first_arg = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (alloc_fn))); if ((TREE_VALUE (first_arg) == ptr_type_node) diff --git a/gcc/testsuite/g++.dg/init/pr35878_1.C b/gcc/testsuite/g++.dg/init/pr35878_1.C index e2fc493..7fb3221 100644 --- a/gcc/testsuite/g++.dg/init/pr35878_1.C +++ b/gcc/testsuite/g++.dg/init/pr35878_1.C @@ -1,7 +1,7 @@ // PR c++/35878 // { dg-do compile } // { dg-options "-O2 -std=gnu++11 -fdump-tree-optimized" } -// { dg-final { scan-tree-dump-times "v_\[0-9]+\\(D\\) \[=!]= 0" 1 "optimized" } } +// { dg-final { scan-tree-dump-not "v_\[0-9]+\\(D\\) \[=!]= 0" "optimized" } } #include <new> #include <utility> diff --git a/gcc/testsuite/g++.dg/init/pr35878_4.C b/gcc/testsuite/g++.dg/init/pr35878_4.C new file mode 100644 index 0000000..bd27565 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/pr35878_4.C @@ -0,0 +1,23 @@ +// PR c++/35878 +// { dg-do compile } +// { dg-options "-O2 -std=gnu++11 -fcheck-new -fdump-tree-optimized" } +// { dg-final { scan-tree-dump-times "v_\[0-9]+\\(D\\) \[=!]= 0" 1 "optimized" } } + +#include <new> +#include <utility> + +struct s1{ + int a; + int b; + int c; +}; + +void f1 (s1 * v, s1&& s) +{ + new (v) s1(std::move(s)); +} + +void f2 (s1 * v, s1&& s) +{ + *v = std::move(s); +} diff --git a/gcc/testsuite/g++.dg/torture/pr48695.C b/gcc/testsuite/g++.dg/torture/pr48695.C index 44e6c77..2f2953d 100644 --- a/gcc/testsuite/g++.dg/torture/pr48695.C +++ b/gcc/testsuite/g++.dg/torture/pr48695.C @@ -1,4 +1,5 @@ // { dg-do run } +/* { dg-options "-fcheck-new" } */ typedef __SIZE_TYPE__ size_t; diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr31146-2.C b/gcc/testsuite/g++.dg/tree-ssa/pr31146-2.C index 500d8b6..8c94423 100644 --- a/gcc/testsuite/g++.dg/tree-ssa/pr31146-2.C +++ b/gcc/testsuite/g++.dg/tree-ssa/pr31146-2.C @@ -20,6 +20,6 @@ double foo (void) return v.a[2]; } -/* -std=c++17 and above doesn't emit operator new () != NULL, so there is +/* GCC 8 doesn't emit operator new () != NULL, so there is nothing to fold anymore. */ -/* { dg-final { scan-tree-dump "Replaced .* != 0B. with .1" "forwprop1" { target c++14_down } } } */ +/* { dg-final { scan-tree-dump-not "Replaced .* != 0B. with .1" "forwprop1" } } */ diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr31146-3.C b/gcc/testsuite/g++.dg/tree-ssa/pr31146-3.C new file mode 100644 index 0000000..9fb5dc1 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/pr31146-3.C @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fcheck-new -fno-tree-vrp -fdump-tree-forwprop1" } */ + +#include <new> + +template <class T> +struct Vec +{ + Vec() + { + for (int i=0; i<3; ++i) + new (&a[i]) T(0); + } + T a[3]; +}; + +double foo (void) +{ + Vec<double> v; + return v.a[2]; +} + +/* GCC 8 emits operator new () != NULL with -fcheck-new. */ +/* { dg-final { scan-tree-dump "Replaced .* != 0B. with .1" "forwprop1" } } */ diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr41428-2.C b/gcc/testsuite/g++.dg/tree-ssa/pr41428-2.C new file mode 100644 index 0000000..7aff519 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/pr41428-2.C @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fcheck-new -fdump-tree-ccp1-details" } */ + +extern "C" void abort (void); +inline void *operator new (__SIZE_TYPE__, void *__p) throw () { return __p; } + +int foo(void) +{ + float f = 0; + int *i = new (&f) int (1); + return *(int *)&f; +} + +/* GCC 8 emits operator new () != NULL with -fcheck-new. */ +/* { dg-final { scan-tree-dump "Folded into: if \\\(1 != 0\\\)" "ccp1" } } */ diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr41428.C b/gcc/testsuite/g++.dg/tree-ssa/pr41428.C index c0a5eb6..f60bc19 100644 --- a/gcc/testsuite/g++.dg/tree-ssa/pr41428.C +++ b/gcc/testsuite/g++.dg/tree-ssa/pr41428.C @@ -11,6 +11,6 @@ int foo(void) return *(int *)&f; } -/* -std=c++17 and above doesn't emit operator new () != NULL, so there is +/* GCC 8 doesn't emit operator new () != NULL, so there is nothing to fold anymore. */ -/* { dg-final { scan-tree-dump "Folded into: if \\\(1 != 0\\\)" "ccp1" { target c++14_down } } } */ +/* { dg-final { scan-tree-dump-not "Folded into: if \\\(1 != 0\\\)" "ccp1" } } */
> 2017-11-13 Ville Voutilainen <ville.voutilainen@gmail.com> > > gcc/ > > Remove the null check from placement new in all modes > * cp/init.c (build_new_1): Don't do a null check for > a namespace-scope non-replaceable placement new > in any mode unless -fcheck-new is provided. Incorrect entry, it should go in gcc/cp/ChangeLog without the cp/ prefix: * init.c (build_new_1): Don't do a null check for a namespace-scope non-replaceable placement new in any mode unless -fcheck-new is provided.
On 13 November 2017 at 14:17, Eric Botcazou <ebotcazou@adacore.com> wrote: >> 2017-11-13 Ville Voutilainen <ville.voutilainen@gmail.com> >> >> gcc/ >> >> Remove the null check from placement new in all modes >> * cp/init.c (build_new_1): Don't do a null check for >> a namespace-scope non-replaceable placement new >> in any mode unless -fcheck-new is provided. > > Incorrect entry, it should go in gcc/cp/ChangeLog without the cp/ prefix: > > * init.c (build_new_1): Don't do a null check for > a namespace-scope non-replaceable placement new > in any mode unless -fcheck-new is provided. Yes, I know. It'll look more like this: 2017-11-13 Ville Voutilainen <ville.voutilainen@gmail.com> gcc/cp Remove the null check from placement new in all modes * init.c (build_new_1): Don't do a null check for a namespace-scope non-replaceable placement new in any mode unless -fcheck-new is provided. testsuite/ Remove the null check from placement new in all modes * g++.dg/init/pr35878_1.C: Adjust. * g++.dg/init/pr35878_4.C: New. * g++.dg/torture/pr48695.C: Adjust. * g++.dg/tree-ssa/pr31146-2.C: Likewise. * g++.dg/tree-ssa/pr31146-3.C: New. * g++.dg/tree-ssa/pr41428-2.C: Likewise. * g++.dg/tree-ssa/pr41428.C: Adjust. ..except that I won't modify the testsuite's changelog, because we don't tend to do that.
On Mon, Nov 13, 2017 at 6:53 AM, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > * g++.dg/tree-ssa/pr31146-2.C: Likewise. > * g++.dg/tree-ssa/pr31146-3.C: New. > * g++.dg/tree-ssa/pr41428-2.C: Likewise. > * g++.dg/tree-ssa/pr41428.C: Adjust. These are testing optimizations, so let's just add -fcheck-new, not additional testcases. OK with that change. Jason
diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 9e6e3af..1fcd91d 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -2758,7 +2758,7 @@ malloc_alignment () static bool std_placement_new_fn_p (tree alloc_fn) { - if ((cxx_dialect > cxx14) && DECL_NAMESPACE_SCOPE_P (alloc_fn)) + if (DECL_NAMESPACE_SCOPE_P (alloc_fn)) { tree first_arg = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (alloc_fn))); if ((TREE_VALUE (first_arg) == ptr_type_node) diff --git a/gcc/testsuite/g++.dg/init/pr35878_1.C b/gcc/testsuite/g++.dg/init/pr35878_1.C index e2fc493..7fb3221 100644 --- a/gcc/testsuite/g++.dg/init/pr35878_1.C +++ b/gcc/testsuite/g++.dg/init/pr35878_1.C @@ -1,7 +1,7 @@ // PR c++/35878 // { dg-do compile } // { dg-options "-O2 -std=gnu++11 -fdump-tree-optimized" } -// { dg-final { scan-tree-dump-times "v_\[0-9]+\\(D\\) \[=!]= 0" 1 "optimized" } } +// { dg-final { scan-tree-dump-not "v_\[0-9]+\\(D\\) \[=!]= 0" "optimized" } } #include <new> #include <utility> diff --git a/gcc/testsuite/g++.dg/init/pr35878_4.C b/gcc/testsuite/g++.dg/init/pr35878_4.C new file mode 100644 index 0000000..bd27565 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/pr35878_4.C @@ -0,0 +1,23 @@ +// PR c++/35878 +// { dg-do compile } +// { dg-options "-O2 -std=gnu++11 -fcheck-new -fdump-tree-optimized" } +// { dg-final { scan-tree-dump-times "v_\[0-9]+\\(D\\) \[=!]= 0" 1 "optimized" } } + +#include <new> +#include <utility> + +struct s1{ + int a; + int b; + int c; +}; + +void f1 (s1 * v, s1&& s) +{ + new (v) s1(std::move(s)); +} + +void f2 (s1 * v, s1&& s) +{ + *v = std::move(s); +}
Tested manually on Linux-X64, finishing testing with the full suite on Linux-PPC64. I spent far too much time contemplating whether to add a compatibility switch for this, but -fcheck-new *is* such a compatibility switch. OK for trunk? 2017-11-10 Ville Voutilainen <ville.voutilainen@gmail.com> gcc/ Remove the null check from placement new in all modes * cp/init.c (build_new_1): Don't do a null check for a namespace-scope non-replaceable placement new in any mode unless -fcheck-new is provided. testsuite/ Remove the null check from placement new in all modes * g++.dg/init/pr35878_1.C: Adjust. * g++.dg/init/pr35878_4.C: New.