On Fri, 22 Nov 2024, Jakub Jelinek wrote:
> On Tue, Nov 19, 2024 at 01:52:06PM +0100, Jan Hubicka wrote:
> > > On Tue, Nov 19, 2024 at 11:23:31AM +0100, Jakub Jelinek wrote:
> > > > On Tue, Nov 19, 2024 at 10:25:16AM +0100, Richard Biener wrote:
> > > > > I think it's pretty clear and easy to describe to users what "m " and
> > > > > what "mC" do. But with "pure" this is an odd intermediate state.
> > > > > For both
> > > > > "m " and "mP" you suggest above the new/delete might modify their
> > > > > global state but as you can't rely on the new/delete pair to prevail
> > > > > you cannot rely on the modification to happen. But how do you explain
> > > > > that
> > > >
> > > > If we are willing to make the default not strictly conforming (i.e.
> > > > basically revert PR101480 by default and make the GCC 11.1/11.2 behavior
> > > > the default and allow -fno-sane-operators-new-delete to change to GCC
> > > > 11.3/14.* behavior), I can live with it.
> > > > But we need to make the documentation clear that the default is not
> > > > strictly
> > > > conforming.
> > >
> > > Here is a modified version of the patch to do that.
> > >
> > > Or do we want to set the default based on -std= option (-std=gnu* implies
> > > -fassume-sane-operators-new-delete, -std=c++* implies
> > > -fno-assume-sane-operators-new-delete)? Though, not sure what to do for
> > > LTO then.
> >
> > My oriignal plan was to add " sane" attribute to the declarations and
> > prevent them from being merged. Then every direct call to new/delete
> > would know if it came from sane or insane translation unit.
> >
> > Alternatively one can also declare
> > +C++ ObjC++ LTO Var(flag_assume_sane_operators_new_delete) Init(1)
> > +Assume C++ replaceable global operators new, new[], delete, delete[]
> > don't read or write visible global state.
> > as optimization. Then sanity would be function specific.
> >
> > inline_call contains code that drops flag_strict_aliasing for function
> > when it inlines -fno-strict-alising function into -fstrict-aliasing.
> > At same place we can make new/delete operator insanity similarly
> > contagious. If you inline function that has insane new/delete calls you
> > make the combined function also insane.
>
> Here is an updated patch, which makes it Optimize and merges it during
> inlining (pessimistically).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> Note, perhaps it could be (maybe incrementally) refined to only clear
> the flag on inlining if the callee actually has any
> gimple_call_from_new_or_delete stmts calling DECL_IS_REPLACEABLE_OPERATOR.
> I.e. like it collects whether a function uses floating point operations
> also gather this in another flag.
After 64bit location_t we have spare flags in gimple, I think we could
reserve a flag to indicate something with that something dependent on
the function called - like in the gimple_call_from_new_or_delete case
whether it's considered "sane" or not. This would solve the inlining
case (we now also have two bits left in GF_CALL, so possible even now)
Thanks,
Richard.
> 2024-11-22 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/110137
> PR middle-end/101480
> gcc/
> * doc/invoke.texi (-fassume-sane-operators-new-delete,
> -fno-assume-sane-operators-new-delete): Document.
> * gimple.cc (gimple_call_fnspec): Handle
> -f{,no-}assume-sane-operators-new-delete.
> * ipa-inline-transform.cc (inline_call): Also clear
> flag_assume_sane_operators_new_delete on caller when inlining
> -fno-assume-sane-operators-new-delete callee into
> -fassume-sane-operators-new-delete caller.
> gcc/c-family/
> * c.opt (fassume-sane-operators-new-delete): New option.
> gcc/testsuite/
> * g++.dg/tree-ssa/pr110137-1.C: New test.
> * g++.dg/tree-ssa/pr110137-2.C: New test.
> * g++.dg/tree-ssa/pr110137-3.C: New test.
> * g++.dg/tree-ssa/pr110137-4.C: New test.
> * g++.dg/torture/pr10148.C: Add -fno-assume-sane-operators-new-delete
> as dg-additional-options.
> * g++.dg/warn/Warray-bounds-16.C: Revert 2021-11-10 changes.
>
> --- gcc/doc/invoke.texi.jj 2024-11-20 14:27:49.257228428 +0100
> +++ gcc/doc/invoke.texi 2024-11-20 14:44:02.819559242 +0100
> @@ -213,7 +213,9 @@ in the following sections.
> @item C++ Language Options
> @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
> @gccoptlist{-fabi-version=@var{n} -fno-access-control
> --faligned-new=@var{n} -fargs-in-order=@var{n} -fchar8_t -fcheck-new
> +-faligned-new=@var{n} -fargs-in-order=@var{n}
> +-fno-assume-sane-operators-new-delete
> +-fchar8_t -fcheck-new
> -fconcepts -fconstexpr-depth=@var{n} -fconstexpr-cache-depth=@var{n}
> -fconstexpr-loop-limit=@var{n} -fconstexpr-ops-limit=@var{n}
> -fno-elide-constructors
> @@ -3164,6 +3166,35 @@ but few users will need to override the
>
> This flag is enabled by default for @option{-std=c++17}.
>
> +@opindex fno-assume-sane-operators-new-delete
> +@opindex fassume-sane-operators-new-delete
> +@item -fno-assume-sane-operators-new
> +The C++ standard allows replacing the global @code{new}, @code{new[]},
> +@code{delete} and @code{delete[]} operators, though a lot of C++ programs
> +don't replace them and just use the implementation provided version.
> +Furthermore, the C++ standard allows omitting those calls if they are
> +made from new or delete expressions (and by extension the same is
> +assumed if @code{__builtin_operator_new} or @code{__builtin_operator_delete}
> +functions are used).
> +This option allows control over some optimizations around calls
> +to those operators.
> +With @code{-fassume-sane-operators-new-delete} option GCC may assume that
> +calls to the replaceable global operators from new or delete expressions or
> +from @code{__builtin_operator_new} or @code{__builtin_operator_delete} calls
> +don't read or modify any global variables or variables whose address could
> +escape to the operators (global state; except for @code{errno} for the
> +@code{new} and @code{new[]} operators).
> +This allows most optimizations across those calls and is something that
> +the implementation provided operators satisfy unless @code{malloc}
> +implementation details are observable in the code or unless @code{malloc}
> +hooks are used, but might not be satisfied if a program replaces those
> +operators. This behavior is enabled by default.
> +With @code{-fno-assume-sane-operators-new-delete} option GCC must
> +assume all these calls (whether from new or delete expressions or called
> +directly) may read and write global state unless proven otherwise (e.g.@:
> +when GCC compiles their implementation). Use this option if those
> +operators are or may be replaced and code needs to expect such behavior.
> +
> @opindex fchar8_t
> @opindex fno-char8_t
> @item -fchar8_t
> --- gcc/gimple.cc.jj 2024-11-19 12:26:46.270255649 +0100
> +++ gcc/gimple.cc 2024-11-20 14:44:02.820559228 +0100
> @@ -1600,12 +1600,22 @@ gimple_call_fnspec (const gcall *stmt)
> && DECL_IS_OPERATOR_DELETE_P (fndecl)
> && DECL_IS_REPLACEABLE_OPERATOR (fndecl)
> && gimple_call_from_new_or_delete (stmt))
> - return ". o ";
> + {
> + if (flag_assume_sane_operators_new_delete)
> + return ".co ";
> + else
> + return ". o ";
> + }
> /* Similarly operator new can be treated as malloc. */
> if (fndecl
> && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
> && gimple_call_from_new_or_delete (stmt))
> - return "m ";
> + {
> + if (flag_assume_sane_operators_new_delete)
> + return "mC";
> + else
> + return "m ";
> + }
> return "";
> }
>
> --- gcc/ipa-inline-transform.cc.jj 2024-10-25 10:00:29.478767714 +0200
> +++ gcc/ipa-inline-transform.cc 2024-11-20 14:52:12.062738769 +0100
> @@ -391,17 +391,33 @@ inline_call (struct cgraph_edge *e, bool
> = DECL_FUNCTION_PERSONALITY (callee->decl);
>
> bool reload_optimization_node = false;
> - if (!opt_for_fn (callee->decl, flag_strict_aliasing)
> - && opt_for_fn (to->decl, flag_strict_aliasing))
> + bool remove_strict_aliasing
> + = (!opt_for_fn (callee->decl, flag_strict_aliasing)
> + && opt_for_fn (to->decl, flag_strict_aliasing));
> + bool remove_assume_sane_operators_new_delete
> + = (!opt_for_fn (callee->decl, flag_assume_sane_operators_new_delete)
> + && opt_for_fn (to->decl, flag_assume_sane_operators_new_delete));
> + if (remove_strict_aliasing || remove_assume_sane_operators_new_delete)
> {
> struct gcc_options opts = global_options;
> struct gcc_options opts_set = global_options_set;
>
> cl_optimization_restore (&opts, &opts_set, opts_for_fn (to->decl));
> - opts.x_flag_strict_aliasing = false;
> - if (dump_file)
> - fprintf (dump_file, "Dropping flag_strict_aliasing on %s\n",
> - to->dump_name ());
> + if (remove_strict_aliasing)
> + {
> + opts.x_flag_strict_aliasing = false;
> + if (dump_file)
> + fprintf (dump_file, "Dropping flag_strict_aliasing on %s\n",
> + to->dump_name ());
> + }
> + if (remove_assume_sane_operators_new_delete)
> + {
> + opts.x_flag_assume_sane_operators_new_delete = false;
> + if (dump_file)
> + fprintf (dump_file,
> + "Dropping flag_assume_sane_operators_new_delete on %s\n",
> + to->dump_name ());
> + }
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION (to->decl)
> = build_optimization_node (&opts, &opts_set);
> reload_optimization_node = true;
> --- gcc/c-family/c.opt.jj 2024-11-19 12:26:46.263255748 +0100
> +++ gcc/c-family/c.opt 2024-11-20 14:45:02.092732924 +0100
> @@ -1690,6 +1690,10 @@ fasm
> C ObjC C++ ObjC++ Var(flag_no_asm, 0)
> Recognize the \"asm\" keyword.
>
> +fassume-sane-operators-new-delete
> +C++ ObjC++ Optimization Var(flag_assume_sane_operators_new_delete) Init(1)
> +Assume C++ replaceable global operators new, new[], delete, delete[] don't
> read or write visible global state.
> +
> ; Define extra predefined macros for use in libgcc.
> fbuilding-libgcc
> C ObjC C++ ObjC++ Undocumented Var(flag_building_libgcc)
> --- gcc/testsuite/g++.dg/tree-ssa/pr110137-1.C.jj 2024-11-20
> 14:44:02.820559228 +0100
> +++ gcc/testsuite/g++.dg/tree-ssa/pr110137-1.C 2024-11-20
> 14:44:02.820559228 +0100
> @@ -0,0 +1,74 @@
> +// PR c++/110137
> +// { dg-do compile }
> +// { dg-options "-O2 -fdump-tree-optimized" }
> +// { dg-final { scan-tree-dump "j = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump "m = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "q = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "t = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "k = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-times "r = 1;" 2 "optimized" } } */
> +
> +int i, j, k, l, m, n, o, q, r, s, t, u;
> +
> +void *
> +foo ()
> +{
> + i = 1;
> + j = 2;
> + void *p = ::operator new (32);
> + j = 3;
> + k = i;
> + return p;
> +}
> +
> +void
> +bar (void *p)
> +{
> + l = 1;
> + m = 2;
> + ::operator delete (p);
> + m = 3;
> + n = l;
> +}
> +
> +int *
> +baz ()
> +{
> + o = 1;
> + q = 2;
> + int *p = new int;
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +void
> +qux (int *p)
> +{
> + s = 1;
> + t = 2;
> + delete p;
> + t = 3;
> + u = s;
> +}
> +
> +void *
> +corge ()
> +{
> + o = 1;
> + q = 2;
> + void *p = __builtin_operator_new (32);
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +void
> +waldo (void *p)
> +{
> + s = 1;
> + t = 2;
> + __builtin_operator_delete (p);
> + t = 3;
> + u = s;
> +}
> --- gcc/testsuite/g++.dg/tree-ssa/pr110137-2.C.jj 2024-11-20
> 14:44:02.820559228 +0100
> +++ gcc/testsuite/g++.dg/tree-ssa/pr110137-2.C 2024-11-20
> 14:44:02.820559228 +0100
> @@ -0,0 +1,74 @@
> +// PR c++/110137
> +// { dg-do compile }
> +// { dg-options "-O2 -fdump-tree-optimized
> -fassume-sane-operators-new-delete" }
> +// { dg-final { scan-tree-dump "j = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump "m = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "q = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "t = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "k = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-times "r = 1;" 2 "optimized" } } */
> +
> +int i, j, k, l, m, n, o, q, r, s, t, u;
> +
> +void *
> +foo ()
> +{
> + i = 1;
> + j = 2;
> + void *p = ::operator new (32);
> + j = 3;
> + k = i;
> + return p;
> +}
> +
> +void
> +bar (void *p)
> +{
> + l = 1;
> + m = 2;
> + ::operator delete (p);
> + m = 3;
> + n = l;
> +}
> +
> +int *
> +baz ()
> +{
> + o = 1;
> + q = 2;
> + int *p = new int;
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +void
> +qux (int *p)
> +{
> + s = 1;
> + t = 2;
> + delete p;
> + t = 3;
> + u = s;
> +}
> +
> +void *
> +corge ()
> +{
> + o = 1;
> + q = 2;
> + void *p = __builtin_operator_new (32);
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +void
> +waldo (void *p)
> +{
> + s = 1;
> + t = 2;
> + __builtin_operator_delete (p);
> + t = 3;
> + u = s;
> +}
> --- gcc/testsuite/g++.dg/tree-ssa/pr110137-3.C.jj 2024-11-20
> 14:44:02.821559214 +0100
> +++ gcc/testsuite/g++.dg/tree-ssa/pr110137-3.C 2024-11-20
> 14:44:02.821559214 +0100
> @@ -0,0 +1,76 @@
> +// PR c++/110137
> +// { dg-do compile }
> +// { dg-options "-O2 -fdump-tree-optimized
> -fno-assume-sane-operators-new-delete" }
> +// { dg-final { scan-tree-dump "j = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump "m = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-times "q = 2;" 2 "optimized" } } */
> +// { dg-final { scan-tree-dump-times "t = 2;" 2 "optimized" } } */
> +// { dg-final { scan-tree-dump-not "k = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "n = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "r = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "u = 1;" "optimized" } } */
> +
> +int i, j, k, l, m, n, o, q, r, s, t, u;
> +
> +void *
> +foo ()
> +{
> + i = 1;
> + j = 2;
> + void *p = ::operator new (32);
> + j = 3;
> + k = i;
> + return p;
> +}
> +
> +void
> +bar (void *p)
> +{
> + l = 1;
> + m = 2;
> + ::operator delete (p);
> + m = 3;
> + n = l;
> +}
> +
> +int *
> +baz ()
> +{
> + o = 1;
> + q = 2;
> + int *p = new int;
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +void
> +qux (int *p)
> +{
> + s = 1;
> + t = 2;
> + delete p;
> + t = 3;
> + u = s;
> +}
> +
> +void *
> +corge ()
> +{
> + o = 1;
> + q = 2;
> + void *p = __builtin_operator_new (32);
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +void
> +waldo (void *p)
> +{
> + s = 1;
> + t = 2;
> + __builtin_operator_delete (p);
> + t = 3;
> + u = s;
> +}
> --- gcc/testsuite/g++.dg/tree-ssa/pr110137-4.C.jj 2024-11-20
> 14:53:59.912235254 +0100
> +++ gcc/testsuite/g++.dg/tree-ssa/pr110137-4.C 2024-11-20
> 15:00:07.162114734 +0100
> @@ -0,0 +1,124 @@
> +// PR c++/110137
> +// { dg-do compile }
> +// { dg-options "-O2 -fdump-tree-optimized" }
> +// { dg-final { scan-tree-dump "j = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump "m = 2;" "optimized" } } */
> +// { dg-final { scan-tree-dump-times "q = 2;" 2 "optimized" } } */
> +// { dg-final { scan-tree-dump-times "t = 2;" 2 "optimized" } } */
> +// { dg-final { scan-tree-dump-not "k = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "n = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "r = 1;" "optimized" } } */
> +// { dg-final { scan-tree-dump-not "u = 1;" "optimized" } } */
> +
> +int i, j, k, l, m, n, o, q, r, s, t, u;
> +
> +static inline
> +__attribute__((always_inline,
> + optimize ("no-assume-sane-operators-new-delete"))) void *
> +foo ()
> +{
> + i = 1;
> + j = 2;
> + void *p = ::operator new (32);
> + j = 3;
> + k = i;
> + return p;
> +}
> +
> +static inline
> +__attribute__((always_inline,
> + optimize ("no-assume-sane-operators-new-delete"))) void
> +bar (void *p)
> +{
> + l = 1;
> + m = 2;
> + ::operator delete (p);
> + m = 3;
> + n = l;
> +}
> +
> +static inline
> +__attribute__((always_inline,
> + optimize ("no-assume-sane-operators-new-delete"))) int *
> +baz ()
> +{
> + o = 1;
> + q = 2;
> + int *p = new int;
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +static inline
> +__attribute__((always_inline,
> + optimize ("no-assume-sane-operators-new-delete"))) void
> +qux (int *p)
> +{
> + s = 1;
> + t = 2;
> + delete p;
> + t = 3;
> + u = s;
> +}
> +
> +static inline
> +__attribute__((always_inline,
> + optimize ("no-assume-sane-operators-new-delete"))) void *
> +corge ()
> +{
> + o = 1;
> + q = 2;
> + void *p = __builtin_operator_new (32);
> + q = 3;
> + r = o;
> + return p;
> +}
> +
> +static inline
> +__attribute__((always_inline,
> + optimize ("no-assume-sane-operators-new-delete"))) void
> +waldo (void *p)
> +{
> + s = 1;
> + t = 2;
> + __builtin_operator_delete (p);
> + t = 3;
> + u = s;
> +}
> +
> +void *
> +foo2 ()
> +{
> + return foo ();
> +}
> +
> +void
> +bar2 (void *p)
> +{
> + bar (p);
> +}
> +
> +int *
> +baz2 ()
> +{
> + return baz ();
> +}
> +
> +void
> +qux2 (int *p)
> +{
> + qux (p);
> +}
> +
> +void *
> +corge2 ()
> +{
> + return corge ();
> +}
> +
> +void
> +waldo2 (void *p)
> +{
> + waldo (p);
> +}
> --- gcc/testsuite/g++.dg/torture/pr10148.C.jj 2024-11-19 12:26:46.283255466
> +0100
> +++ gcc/testsuite/g++.dg/torture/pr10148.C 2024-11-20 14:44:02.848558838
> +0100
> @@ -1,5 +1,6 @@
> /* { dg-do run } */
> /* { dg-skip-if "requires hosted libstdc++ for stdlib malloc" { ! hostedlib
> } } */
> +/* { dg-additional-options "-fno-assume-sane-operators-new-delete" } */
>
> #include <stdlib.h>
> #include <assert.h>
> --- gcc/testsuite/g++.dg/warn/Warray-bounds-16.C.jj 2023-03-30
> 16:32:03.824853782 +0200
> +++ gcc/testsuite/g++.dg/warn/Warray-bounds-16.C 2024-11-20
> 17:37:09.767970243 +0100
> @@ -19,11 +19,11 @@ struct S
> p = (int*) new unsigned char [sizeof (int) * m];
>
> for (int i = 0; i < m; i++)
> - new (p + i) int (); /* { dg-bogus "bounds" "pr102690" { xfail *-*-* }
> } */
> + new (p + i) int ();
> }
> };
>
> S a (0);
>
> -/* The loop cannot be eliminated since the global 'new' can change 'm'. */
> -/* { dg-final { scan-tree-dump-not "goto" "optimized" { xfail *-*-* } } } */
> +/* Verify the loop has been eliminated.
> + { dg-final { scan-tree-dump-not "goto" "optimized" } } */
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
