PATCH: PR 29487

[Mark Mitchell <mark at codesourcery dot com>]

This patch fixed (I think) PR 29487, the last critical GCC 4.1.2
issue.  The PR is caused by a target-independent pessimization on all
targets for code using exception handling.  In particular, a function
which does not throw any exceptions, but which is COMDAT, is now
emitted with EH information, and the optimizers conservatively assume
that it may throw exceptions.

The reason this change was made was for the benefit of functions
explicitly declared weak; such functions may be replaced by the user
at link-time, and we must therefore be conservative in making
assumptions about them.  The same is not true for COMDAT functions,
since all COMDAT functions must be functionally equivalent.

I have attacked the problem by first adding a macro DECL_REPLACEABLE_P
which is true iff a function may be replaced at link-time with a
functionally different definition.  This macro is like binds_local_p
-- but more agressive; while a COMDAT function is not binds_local_p,
it is also not DECL_REPLACEABLE_P.  The macro is used in the several
places in the compiler that have been identified as gating the setting
of TREE_NOTHROW.

I did not attempt to fix the other problem from PR 29487, which is
that weak functions which do not throw exceptions are now emitted with
exception unwind information.  That problem stems from the dual use of
TREE_NOTHROW.  Because TREE_NOTHROW is used to indicate to the
optimizers that a function does not throw, setting it for a
replaceable function might confuse them.  But, TREE_NOTHROW is also
used to determine whether or not unwind information should be emitted
for a function.  I think that we don't really need a separate bit: we
just need the optimizers to check "TREE_NOTHROW (callee) &&
!DECL_REPLACEABLE (callee)", but that was a bigger change, and I
wanted to be conservative.

I have not yet fully tested this patch, but I am running tests now.  I
wanted to post the patch early for comments, and so that Dave could
try it on HP-UX 10.10, which was the system originally mentioned in
the PR.  And, I wanted Joern and others to have a chance to comment.

If all goes well, I will check this in soon, and build GCC 4.1.2 RC2.
Then, I will apply the same patch to 4.2 and mainline, with suitable
changes, if required.

Comments?

Thanks,

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

2007-02-06  Mark Mitchell  <mark@codesourcery.com>

	PR target/29487
	* tree.h (DECL_REPLACEABLE_P): New macro.
	* except.c (set_nothrow_function_flags): Likewise.

2007-02-06  Mark Mitchell  <mark@codesourcery.com>

	PR target/29487
	* decl.c (finish_function): Use DECL_REPLACEABLE.
	* tree.c (cp_cannot_inline_tree_fn): Likewise.

2007-02-06  Mark Mitchell  <mark@codesourcery.com>

	PR c++/29487
	* g++.dg/eh/weak1-C: New test.
	* g++.dg/eh/weak1-a.cc: Likewise.
	* g++.dg/eh/comdat1.C: Likewise.

Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 121662)
+++ gcc/tree.h	(working copy)
@@ -2452,6 +2452,18 @@ extern void decl_restrict_base_insert (t
    something which is DECL_COMDAT.  */
 #define DECL_COMDAT(NODE) (DECL_WITH_VIS_CHECK (NODE)->decl_with_vis.comdat_flag)
 
+/* A replaceable function is one which may be replaced at link-time
+   with an entirely different definition, provided that the
+   replacement has the same type.  For example, functions declared
+   with __attribute__((weak)) on most systems are replaceable.  
+
+   COMDAT functions are not replaceable, since all definitions of the
+   function must be equivalent.  It is important that COMDAT functions
+   not be treated as replaceable so that use of C++ template
+   instantiations  is not pessimized.  */
+#define DECL_REPLACEABLE_P(NODE) \
+  (!DECL_COMDAT (NODE) && !targetm.binds_local_p (NODE))
+
 /* The name of the object as the assembler will see it (but before any
    translations made by ASM_OUTPUT_LABELREF).  Often this is the same
    as DECL_NAME.  It is an IDENTIFIER_NODE.  */
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 121662)
+++ gcc/cp/decl.c	(working copy)
@@ -11010,7 +11010,7 @@ finish_function (int flags)
   if (!processing_template_decl
       && !cp_function_chain->can_throw
       && !flag_non_call_exceptions
-      && targetm.binds_local_p (fndecl))
+      && !DECL_REPLACEABLE_P (fndecl))
     TREE_NOTHROW (fndecl) = 1;
 
   /* This must come after expand_function_end because cleanups might
Index: gcc/cp/tree.c
===================================================================
--- gcc/cp/tree.c	(revision 121662)
+++ gcc/cp/tree.c	(working copy)
@@ -2038,13 +2038,9 @@ cp_cannot_inline_tree_fn (tree* fnp)
       && lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)) == NULL)
     return 1;
 
-  /* Don't auto-inline anything that might not be bound within
-     this unit of translation.
-     Exclude comdat functions from this rule.  While they can be bound
-     to the other unit, they all must be the same.  This is especially
-     important so templates can inline.  */
-  if (!DECL_DECLARED_INLINE_P (fn) && !(*targetm.binds_local_p) (fn)
-      && !DECL_COMDAT (fn))
+  /* Don't auto-inline functions that might be replaced at link-time
+     with an alternative definition.  */ 
+  if (!DECL_DECLARED_INLINE_P (fn) && DECL_REPLACEABLE_P (fn))
     {
       DECL_UNINLINABLE (fn) = 1;
       return 1;
Index: gcc/except.c
===================================================================
--- gcc/except.c	(revision 121662)
+++ gcc/except.c	(working copy)
@@ -2701,7 +2701,10 @@ set_nothrow_function_flags (void)
 {
   rtx insn;
 
-  if (!targetm.binds_local_p (current_function_decl))
+  /* If we don't know that this implementation of the function will
+     actually be used, then we must not set TREE_NOTHROW, since
+     callers must not assume that this function does not throw.  */
+  if (DECL_REPLACEABLE_P (current_function_decl))
     return;
 
   TREE_NOTHROW (current_function_decl) = 1;
Index: gcc/testsuite/g++.dg/eh/weak1-a.cc
===================================================================
--- gcc/testsuite/g++.dg/eh/weak1-a.cc	(revision 0)
+++ gcc/testsuite/g++.dg/eh/weak1-a.cc	(revision 0)
@@ -0,0 +1,3 @@
+extern void f() {
+  throw 7;
+}
Index: gcc/testsuite/g++.dg/eh/weak1.C
===================================================================
--- gcc/testsuite/g++.dg/eh/weak1.C	(revision 0)
+++ gcc/testsuite/g++.dg/eh/weak1.C	(revision 0)
@@ -0,0 +1,22 @@
+// PR target/29487
+// { dg-do run }
+// { dg-additional-sources "weak1-a.cc" }
+// { dg-options "-O2" }
+
+extern __attribute__((weak)) 
+void f() {
+}
+
+int main () {
+  try {
+    f();
+    return 1;
+  } catch (int i) {
+    /* Although the implementation of f in this file does not throw
+       any exceptions, it is weak, and may therefore be replaced at
+       link time.  Therefore, the compiler must not optimize away this
+       catch clause.  */
+    if (i != 7)
+      return 2;
+  }
+}
Index: gcc/testsuite/g++.dg/eh/comdat1.C
===================================================================
--- gcc/testsuite/g++.dg/eh/comdat1.C	(revision 0)
+++ gcc/testsuite/g++.dg/eh/comdat1.C	(revision 0)
@@ -0,0 +1,42 @@
+// PR target/29487
+// { dg-do link }
+// { dg-options "-O2" }
+
+/* This function is not defined.  The compiler should optimize away
+   all calls to it.  */
+extern void undefined () throw ();
+
+extern void f1();
+
+inline void f2() {
+  f1();
+}
+
+/* This function will be COMDAT if not inlined.  */
+inline void f1() {}
+
+/* This function will be COMDAT.  */
+template <typename T>
+void f3() {
+  if (false)
+    throw 3;
+}
+
+inline void f4() {
+  if (false)
+    throw 7;
+}
+
+int main () {
+  try {
+    f1();
+    f2();
+    f3<int>();
+    f4();
+  } catch (...) {
+    /* The compiler should recognize that none of the functions above
+       can throw exceptions, and therefore remove this code as
+       unreachable.  */
+    undefined ();
+  }
+}