On Tue, Nov 21, 2017 at 12:14 PM, Dominik Inführ <dominik.infu...@theobroma-systems.com> wrote: > Hi, > > this patch tries to extend tree-ssa-dce.c to remove unnecessary > new/delete-pairs (it already does that for malloc/free). Clang does it too > and it seems to be allowed by > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3664.html. I’ve > bootstrapped/regtested on aarch64-linux and x86_64-linux.
Few comments. +/* Return true when STMT is operator new call. */ + +bool +gimple_call_operator_new_p (const gimple *stmt) +{ + tree fndecl; + + if (is_gimple_call (stmt) + && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE) + return DECL_IS_OPERATOR_NEW (fndecl); + return false; make these take a gcall * please and drop the is_gimple_call check. diff --git a/gcc/tree-core.h b/gcc/tree-core.h index aa54221253c..9a406c5d86c 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -1787,7 +1787,9 @@ struct GTY(()) tree_function_decl { unsigned has_debug_args_flag : 1; unsigned tm_clone_flag : 1; unsigned versioned_function : 1; - /* No bits left. */ + + unsigned operator_delete_flag : 1; + /* 31 bits left. */ while it looks bad reality is that on 64bit pointer hosts we had 32 bits left. Note that we could really pack things better and even make function_decl smaller by using the tail-padding (for 64bit pointer hosts...) in tree_decl_with_vis which also has 32 + 14 bits left. + /* For operator delete [] we need to keep size operand + alive. Otherwise this operand isn't available anymore + when we finally decide that this stmt is necessary in + eliminate_unnecessary_stmts. If it should really be + unnecessary, a later pass can clean this up. */ + if (gimple_call_operator_delete_p (stmt)) + { + for (unsigned i=1; i < gimple_call_num_args (stmt); ++i) + { + tree arg = gimple_call_arg (stmt, i); + if (TREE_CODE (arg) == SSA_NAME) + mark_operand_necessary (arg); + } + } bah ... so the "hack" for malloc/free pair removal doesn't really work very well for new/delete. What you do looks reasonable but in the end we might support sth like "tentatively not necessary" and a late marking things necessary that have been proven to be necessary anyway. diff --git a/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc b/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc index bc36ed595e0..10e26615fb7 100644 --- a/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc +++ b/libstdc++-v3/testsuite/ext/bitmap_allocator/check_delete.cc @@ -15,6 +15,8 @@ // with this library; see the file COPYING3. If not see // <http://www.gnu.org/licenses/>. +// { dg-options "-fno-tree-dce" } + // 20.4.1.1 allocator members hmm. I think it would be better to add a new C++ FE option -fno-allocation-dce that simply doesn't set DECL_IS_OPERATOR_{NEW,DELETE} similar to how we have -fno-lifetime-dse? There might be projects that are not happy with this DCE and disabling _all_ DCE just to keep existing behavior looks bad. Otherwise the patch looks straight-forward in case C++ folks are happy with it. Thanks for working on this! I think that if you manage to convince C++ folks and fix the -fno-allocation-dce missing then I'd like to have this in early stage3. We've been requesting this for a long time. Richard. > Best, > Dominik >