public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] DCE __cxa_atexit calls where the function is pure/const [PR19661]
@ 2024-05-02 21:56 Andrew Pinski
  2024-05-03 14:53 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Pinski @ 2024-05-02 21:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

In C++ sometimes you have a deconstructor function which is "empty", like for an
example with unions or with arrays.  The front-end might not know it is empty either
so this should be done on during optimization.o
To implement it I added it to DCE where we mark if a statement is necessary or not.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

	PR tree-optimization/19661

gcc/ChangeLog:

	* tree-ssa-dce.cc (is_cxa_atexit): New function.
	(is_removable_cxa_atexit_call): New function.
	(mark_stmt_if_obviously_necessary): Don't mark removable
	cxa_at_exit calls.
	(mark_all_reaching_defs_necessary_1): Likewise.
	(propagate_necessity): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/tree-ssa/cxa_atexit-1.C: New test.
	* g++.dg/tree-ssa/cxa_atexit-2.C: New test.
	* g++.dg/tree-ssa/cxa_atexit-3.C: New test.
	* g++.dg/tree-ssa/cxa_atexit-4.C: New test.
	* g++.dg/tree-ssa/cxa_atexit-5.C: New test.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-1.C | 20 +++++++++
 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-2.C | 21 ++++++++++
 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-3.C | 19 +++++++++
 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-4.C | 20 +++++++++
 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-5.C | 39 +++++++++++++++++
 gcc/tree-ssa-dce.cc                          | 44 ++++++++++++++++++++
 6 files changed, 163 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-1.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-2.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-3.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-4.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-5.C

diff --git a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-1.C b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-1.C
new file mode 100644
index 00000000000..1f5f431c7e4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-1.C
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-cddce1-details -fdump-tree-optimized" } */
+// { dg-require-effective-target cxa_atexit }
+/* PR tree-optimization/19661 */
+
+/* The call to axexit should be removed as A::~A() is a pure/const function call
+   and there is no visible effect if A::~A() call does not happen.  */
+
+struct A { 
+    A(); 
+    ~A() {} 
+}; 
+ 
+void foo () { 
+  static A a; 
+} 
+
+/* { dg-final { scan-tree-dump-times "Deleting : __cxxabiv1::__cxa_atexit" 1 "cddce1" } } */
+/* { dg-final { scan-tree-dump-not "__cxa_atexit" "optimized" } } */
+
diff --git a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-2.C b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-2.C
new file mode 100644
index 00000000000..4d0656b455c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-2.C
@@ -0,0 +1,21 @@
+/* { dg-do compile { target c++11 } } */
+/* { dg-options "-O2 -fdump-tree-cddce1-details -fdump-tree-optimized" } */
+// { dg-require-effective-target cxa_atexit }
+/* PR tree-optimization/19661 */
+
+/* The call to axexit should be not removed as A::~A() as it marked with noipa.  */
+
+struct A { 
+    A(); 
+    ~A();
+}; 
+
+[[gnu::noipa]] A::~A() {}
+ 
+void foo () { 
+  static A a; 
+} 
+
+/* { dg-final { scan-tree-dump-not "Deleting : __cxxabiv1::__cxa_atexit" "cddce1" } } */
+/* { dg-final { scan-tree-dump-times "__cxa_atexit" 1 "optimized" } } */
+
diff --git a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-3.C b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-3.C
new file mode 100644
index 00000000000..03a19209661
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-3.C
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-cddce1-details -fdump-tree-optimized" } */
+// { dg-require-effective-target cxa_atexit }
+/* PR tree-optimization/19661 */
+
+/* We should not remove the call to atexit as A::~A is unknown.  */
+
+struct A { 
+    A(); 
+    ~A();
+}; 
+
+void foo () { 
+  static A a; 
+} 
+
+/* { dg-final { scan-tree-dump-not "Deleting : __cxxabiv1::__cxa_atexit" "cddce1" } } */
+/* { dg-final { scan-tree-dump-times "__cxa_atexit" 1 "optimized" } } */
+
diff --git a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-4.C b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-4.C
new file mode 100644
index 00000000000..b85a7efd16b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-4.C
@@ -0,0 +1,20 @@
+/* { dg-do compile { target c++11 } } */
+/* { dg-options "-O2 -fdump-tree-cddce1-details -fdump-tree-optimized -w" } */
+// { dg-require-effective-target cxa_atexit }
+/* PR tree-optimization/19661 */
+
+/* The call to axexit should be removed as A::~A() is a pure/const function call
+   and there is no visible effect if A::~A() call does not happen.  */
+
+struct A { 
+    A(); 
+    [[gnu::pure]] ~A();
+}; 
+ 
+void foo () { 
+  static A a; 
+} 
+
+/* { dg-final { scan-tree-dump-times "Deleting : __cxxabiv1::__cxa_atexit" 1 "cddce1" } } */
+/* { dg-final { scan-tree-dump-not "__cxa_atexit" "optimized" } } */
+
diff --git a/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-5.C b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-5.C
new file mode 100644
index 00000000000..f96395df913
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cxa_atexit-5.C
@@ -0,0 +1,39 @@
+/* { dg-do compile { target c++20 } } */
+/* { dg-options "-O2 -fdump-tree-dce2-details -fdump-tree-optimized" } */
+// { dg-require-effective-target cxa_atexit }
+/* PR tree-optimization/19661 */
+
+/* The call to axexit should be removed as constant_init::~constant_init is a pure/const function call
+   and there is no visible effect if constant_init::~constant_init() call does not happen.  */
+/* This takes until DCE2 as constant_init::~constant_init is not figured out being pure/const until late. */
+
+extern "C" int puts(const char*);
+
+struct A
+{
+  constexpr A()  { }
+  ~A() { puts("bye"); }
+};
+
+namespace
+{
+  struct constant_init
+  {
+    union {
+      A obj;
+    };
+    constexpr constant_init() : obj() { }
+
+    ~constant_init() { /* do nothing, union member is not destroyed */ }
+  };
+
+
+  // Single-threaded fallback buffer.
+  constinit constant_init global;
+}
+
+extern "C" A* get() { return &global.obj; }
+
+/* { dg-final { scan-tree-dump-times "Deleting : __cxxabiv1::__cxa_atexit" 1 "dce2" } } */
+/* { dg-final { scan-tree-dump-not "__cxa_atexit" "optimized" } } */
+
diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index 636c471d4c8..762218a04fe 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -124,6 +124,41 @@ keep_all_vdefs_p ()
   return optimize_debug;
 }
 
+/* True if CALLEE is the function __cxa_atexit. */
+
+static inline bool
+is_cxa_atexit (const_tree callee)
+{
+  if (callee != NULL_TREE
+      && strcmp (IDENTIFIER_POINTER (DECL_NAME (callee)), "__cxa_atexit") == 0)
+    return true;
+  return false;
+}
+
+/* True if STMT is a call to __cxa_atexit and the first argument to that
+   call is a pure/const non looping function decl.  */
+
+static inline bool
+is_removable_cxa_atexit_call (gimple *stmt)
+{
+  tree callee = gimple_call_fndecl (stmt);
+  if (!is_cxa_atexit (callee))
+    return false;
+  if (gimple_call_num_args (stmt) != 3)
+    return false;
+  tree arg = gimple_call_arg (stmt, 0);
+  if (TREE_CODE (arg) != ADDR_EXPR)
+    return false;
+  arg = TREE_OPERAND (arg, 0);
+  if (TREE_CODE (arg) != FUNCTION_DECL)
+    return false;
+  int flags = flags_from_decl_or_type (arg);
+  if (flags & ECF_NORETURN)
+    return false;
+  return (flags & (ECF_CONST|ECF_PURE))
+	  && !(flags & ECF_LOOPING_CONST_OR_PURE);
+}
+
 /* If STMT is not already marked necessary, mark it, and add it to the
    worklist if ADD_TO_WORKLIST is true.  */
 
@@ -251,6 +286,10 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
 	    && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee))
 	  return;
 
+	/* For __cxa_atexit calls, don't mark as necessary right away. */
+	if (is_removable_cxa_atexit_call (stmt))
+	  return;
+
 	/* IFN_GOACC_LOOP calls are necessary in that they are used to
 	   represent parameter (i.e. step, bound) of a lowered OpenACC
 	   partitioned loop.  But this kind of partitioned loop might not
@@ -626,6 +665,8 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref ATTRIBUTE_UNUSED,
 	      || DECL_IS_OPERATOR_DELETE_P (callee))
 	  && gimple_call_from_new_or_delete (call))
 	return false;
+      if (is_removable_cxa_atexit_call (call))
+	return false;
     }
 
   if (! gimple_clobber_p (def_stmt))
@@ -930,6 +971,9 @@ propagate_necessity (bool aggressive)
 		  && gimple_call_from_new_or_delete (call))
 		continue;
 
+	      if (is_removable_cxa_atexit_call (call))
+		continue;
+
 	      /* Calls implicitly load from memory, their arguments
 	         in addition may explicitly perform memory loads.  */
 	      mark_all_reaching_defs_necessary (call);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] DCE __cxa_atexit calls where the function is pure/const [PR19661]
  2024-05-02 21:56 [PATCH] DCE __cxa_atexit calls where the function is pure/const [PR19661] Andrew Pinski
@ 2024-05-03 14:53 ` Jeff Law
  2024-05-03 20:22   ` Andrew Pinski (QUIC)
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2024-05-03 14:53 UTC (permalink / raw)
  To: Andrew Pinski, gcc-patches



On 5/2/24 3:56 PM, Andrew Pinski wrote:
> In C++ sometimes you have a deconstructor function which is "empty", like for an
> example with unions or with arrays.  The front-end might not know it is empty either
> so this should be done on during optimization.o
> To implement it I added it to DCE where we mark if a statement is necessary or not.
> 
> Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> 	PR tree-optimization/19661
> 
> gcc/ChangeLog:
> 
> 	* tree-ssa-dce.cc (is_cxa_atexit): New function.
> 	(is_removable_cxa_atexit_call): New function.
> 	(mark_stmt_if_obviously_necessary): Don't mark removable
> 	cxa_at_exit calls.
> 	(mark_all_reaching_defs_necessary_1): Likewise.
> 	(propagate_necessity): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/tree-ssa/cxa_atexit-1.C: New test.
> 	* g++.dg/tree-ssa/cxa_atexit-2.C: New test.
> 	* g++.dg/tree-ssa/cxa_atexit-3.C: New test.
> 	* g++.dg/tree-ssa/cxa_atexit-4.C: New test.
> 	* g++.dg/tree-ssa/cxa_atexit-5.C: New test.
OK
jeff


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] DCE __cxa_atexit calls where the function is pure/const [PR19661]
  2024-05-03 14:53 ` Jeff Law
@ 2024-05-03 20:22   ` Andrew Pinski (QUIC)
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Pinski (QUIC) @ 2024-05-03 20:22 UTC (permalink / raw)
  To: Jeff Law, Andrew Pinski (QUIC), gcc-patches



> -----Original Message-----
> From: Jeff Law <jeffreyalaw@gmail.com>
> Sent: Friday, May 3, 2024 7:53 AM
> To: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>; gcc-
> patches@gcc.gnu.org
> Subject: Re: [PATCH] DCE __cxa_atexit calls where the function is pure/const
> [PR19661]
> 
> 
> 
> On 5/2/24 3:56 PM, Andrew Pinski wrote:
> > In C++ sometimes you have a deconstructor function which is "empty",
> > like for an example with unions or with arrays.  The front-end might
> > not know it is empty either so this should be done on during
> > optimization.o To implement it I added it to DCE where we mark if a
> statement is necessary or not.
> >
> > Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > 	PR tree-optimization/19661
> >
> > gcc/ChangeLog:
> >
> > 	* tree-ssa-dce.cc (is_cxa_atexit): New function.
> > 	(is_removable_cxa_atexit_call): New function.
> > 	(mark_stmt_if_obviously_necessary): Don't mark removable
> > 	cxa_at_exit calls.
> > 	(mark_all_reaching_defs_necessary_1): Likewise.
> > 	(propagate_necessity): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	* g++.dg/tree-ssa/cxa_atexit-1.C: New test.
> > 	* g++.dg/tree-ssa/cxa_atexit-2.C: New test.
> > 	* g++.dg/tree-ssa/cxa_atexit-3.C: New test.
> > 	* g++.dg/tree-ssa/cxa_atexit-4.C: New test.
> > 	* g++.dg/tree-ssa/cxa_atexit-5.C: New test.
> OK

I have 2 issues reported to me before I pushed this so I am going to fix/check on them before pushing this.
The first one is the testcase fails on arm-linux-eabi since it uses __eabi_atexit rather than __cxa_atexit (I think the order of arguments for that function is slightly different too).
The second one is making sure the function will bind locally (or the user had the attribute on the function).
I should have a new patch Monday or Tuesday.

Thanks,
Andrew Pinski

> jeff


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-05-03 20:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-02 21:56 [PATCH] DCE __cxa_atexit calls where the function is pure/const [PR19661] Andrew Pinski
2024-05-03 14:53 ` Jeff Law
2024-05-03 20:22   ` Andrew Pinski (QUIC)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).