[PR69315] enable finish_function to recurse for constexpr functions

[PR69315] enable finish_function to recurse for constexpr functions

[Alexandre Oliva]

We don't want finish_function to be called recursively from mark_used.
However, it's desirable and necessary to call itself recursively when
performing delayed folding, because that may have to instantiate and
evaluate constexpr template functions.

So, arrange for finish_function to accept being called recursively
during delayed folding, save and restore the controlling variables,
and process the deferred mark_used calls only when the outermost call
completes.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

for  gcc/cp/ChangeLog

	PR c++/69315
	* decl.c (is_folding_function): New variable.
	(finish_function): Test, save and set it.

for  gcc/testsuite/ChangeLog

	PR c++/69315
	* g++.dg/pr69315.C: New.
---
 gcc/cp/decl.c                  |   31 +++++++++++++++++++++++++------
 gcc/testsuite/g++.dg/pr69315.C |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr69315.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index f4604b6..65eff2f 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -227,10 +227,14 @@ struct GTY((for_user)) named_label_entry {
    function, two inside the body of a function in a local class, etc.)  */
 int function_depth;
 
-/* To avoid unwanted recursion, finish_function defers all mark_used calls
-   encountered during its execution until it finishes.  */
+/* To avoid unwanted recursion, finish_function defers all mark_used
+   calls encountered during its execution until it finishes.
+   finish_function refuses to be called recursively, unless the
+   recursion occurs during folding, which often requires instantiating
+   and evaluating template functions.  */
 bool defer_mark_used_calls;
 vec<tree, va_gc> *deferred_mark_used_calls;
+static bool is_folding_function;
 
 /* States indicating how grokdeclarator() should handle declspecs marked
    with __attribute__((deprecated)).  An object declared as
@@ -14528,8 +14532,11 @@ finish_function (int flags)
   if (c_dialect_objc ())
     objc_finish_function ();
 
-  gcc_assert (!defer_mark_used_calls);
+  gcc_assert (!defer_mark_used_calls
+	      || (is_folding_function && DECL_DECLARED_CONSTEXPR_P (fndecl)));
   defer_mark_used_calls = true;
+  bool save_folding_function = is_folding_function;
+  is_folding_function = false;
 
   record_key_method_defined (fndecl);
 
@@ -14636,7 +14643,14 @@ finish_function (int flags)
 
   /* Perform delayed folding before NRV transformation.  */
   if (!processing_template_decl)
-    cp_fold_function (fndecl);
+    {
+      is_folding_function = true;
+      cp_fold_function (fndecl);
+      /* Check that our controlling variables were restored to the
+	 expect state.  */
+      gcc_assert (is_folding_function && defer_mark_used_calls);
+      is_folding_function = false;
+    }
 
   /* Set up the named return value optimization, if we can.  Candidate
      variables are selected in check_return_expr.  */
@@ -14780,8 +14794,13 @@ finish_function (int flags)
   /* Clean up.  */
   current_function_decl = NULL_TREE;
 
-  defer_mark_used_calls = false;
-  if (deferred_mark_used_calls)
+  is_folding_function = save_folding_function;
+  /* Iff we were called recursively for a constexpr function,
+     is_folding_function was just restored to TRUE.  If we weren't
+     called recursively, it was restored to FALSE.  That's just how
+     defer_mark_used_call ought to be set.  */
+  defer_mark_used_calls = is_folding_function;
+  if (!defer_mark_used_calls && deferred_mark_used_calls)
     {
       unsigned int i;
       tree decl;
diff --git a/gcc/testsuite/g++.dg/pr69315.C b/gcc/testsuite/g++.dg/pr69315.C
new file mode 100644
index 0000000..28975b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr69315.C
@@ -0,0 +1,34 @@
+// { dg-do compile }
+// { dg-options "-std=c++11" }
+
+// Template instantiation and evaluation for folding within
+// finish_function may call finish_function recursively.
+// Make sure we don't reject or delay that sort of recursion.
+
+template <bool> struct Iter;
+
+struct Arg {
+  Iter<true> begin();
+  Iter<true> end();
+};
+
+template <bool> struct Iter {
+  int operator*();
+  Iter operator++();
+  template <bool C1, bool C2> friend constexpr bool operator==(Iter<C1>, Iter<C2>);
+  template <bool C1, bool C2> friend constexpr bool operator!=(Iter<C1>, Iter<C2>);
+};
+
+void func(Arg a) {
+  for (auto ch : a) {
+    a.begin() == a.end();
+  }
+}
+
+template <bool C1, bool C2> constexpr bool operator==(Iter<C1>, Iter<C2>) {
+  return true;
+}
+
+template <bool C1, bool C2> constexpr bool operator!=(Iter<C1> a, Iter<C2> b) {
+  return a == b;
+}

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PR69315] enable finish_function to recurse for constexpr functions

[Alexandre Oliva]

On Jan 26, 2016, Alexandre Oliva <aoliva@redhat.com> wrote:

> We don't want finish_function to be called recursively from mark_used.
> However, it's desirable and necessary to call itself recursively when
> performing delayed folding, because that may have to instantiate and
> evaluate constexpr template functions.

> So, arrange for finish_function to accept being called recursively
> during delayed folding, save and restore the controlling variables,
> and process the deferred mark_used calls only when the outermost call
> completes.

> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Ping?

https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02010.html

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PR69315] enable finish_function to recurse for constexpr functions

[Alexandre Oliva]

On Feb 10, 2016, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Jan 26, 2016, Alexandre Oliva <aoliva@redhat.com> wrote:
>> We don't want finish_function to be called recursively from mark_used.
>> However, it's desirable and necessary to call itself recursively when
>> performing delayed folding, because that may have to instantiate and
>> evaluate constexpr template functions.

>> So, arrange for finish_function to accept being called recursively
>> during delayed folding, save and restore the controlling variables,
>> and process the deferred mark_used calls only when the outermost call
>> completes.

>> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

> Ping?

> https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02010.html

Ping?

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Re: [PR69315] enable finish_function to recurse for constexpr functions

[Jason Merrill]

On 01/26/2016 12:11 PM, Alexandre Oliva wrote:
> We don't want finish_function to be called recursively from mark_used.
> However, it's desirable and necessary to call itself recursively when
> performing delayed folding, because that may have to instantiate and
> evaluate constexpr template functions.

Hmm.  If recursion is problematic, we shouldn't do it.  If it isn't 
problematic, we can do away with defer_mark_used_calls.  It doesn't make 
sense to me to allow recursion some of the time.

I tried disabling defer_mark_used_calls, and the only new testsuite 
failure turned out to be fixing a bug on variadic122.C: we were 
deferring a mark_used call in unevaluated context and then calling it in 
evaluated context.

Jakub, you added defer_mark_used_calls for BZ 37189, do you think it's 
still needed?  The testcase passes without it now.

Jason

Re: [PR69315] enable finish_function to recurse for constexpr functions

[Jakub Jelinek]

On Tue, Mar 22, 2016 at 05:28:10PM -0400, Jason Merrill wrote:
> Jakub, you added defer_mark_used_calls for BZ 37189, do you think it's still
> needed?  The testcase passes without it now.

That's a question.  Digging through history, I found:
1) r149750 aka gimplification unit-at-a-time
2) PR48869 that fixed the regression caused by that; so we now should be
   trying to get_copy_ctor/get_dtor for parallel/task already during
   cp-genericize.c
But, looking at this function, e.g. taskloop construct isn't handled,
will try to construct a testcase where it could matter.  Dunno about OpenACC
or Cilk+ constructs, those aren't handled during cp-gimplify.c either.
Though, as we have gimplification unit-at-a-time, I'd say that even if we
synthetize_method during gimplification of something, we'd still enqueue it
in cgraph for later analysis instead of gimplification right away.
The gimplifier still asserts that it isn't called recursively, so I'd hope
the patch can be reverted (test kept).

	Jakub

Re: [PR69315] enable finish_function to recurse for constexpr functions

[Jakub Jelinek]

On Wed, Mar 23, 2016 at 02:57:36PM +0100, Jakub Jelinek wrote:
> On Tue, Mar 22, 2016 at 05:28:10PM -0400, Jason Merrill wrote:
> > Jakub, you added defer_mark_used_calls for BZ 37189, do you think it's still
> > needed?  The testcase passes without it now.
> 
> That's a question.  Digging through history, I found:
> 1) r149750 aka gimplification unit-at-a-time
> 2) PR48869 that fixed the regression caused by that; so we now should be
>    trying to get_copy_ctor/get_dtor for parallel/task already during
>    cp-genericize.c
> But, looking at this function, e.g. taskloop construct isn't handled,
> will try to construct a testcase where it could matter.  Dunno about OpenACC
> or Cilk+ constructs, those aren't handled during cp-gimplify.c either.
> Though, as we have gimplification unit-at-a-time, I'd say that even if we
> synthetize_method during gimplification of something, we'd still enqueue it
> in cgraph for later analysis instead of gimplification right away.
> The gimplifier still asserts that it isn't called recursively, so I'd hope
> the patch can be reverted (test kept).

I'll test then following (had to tweak the testcase a little bit, put c++11
into target, move it to proper dir, and build with -O2 (as starting with
r233671 we don't ICE on the testcase when -fno-inline, which is the default
at -O0).

2016-03-23  Alexandre Oliva  <aoliva@redhat.com>
	    Jason Merrill  <jason@redhat.com>
	    Jakub Jelinek  <jakub@redhat.com>

	PR c++/69315
	* cp-tree.h (defer_mark_used_calls, deferred_mark_used_calls): Remove.
	* decl.c (defer_mark_used_calls, deferred_mark_used_calls): Remove.
	(finish_function): Don't set or test them.
	* decl2.c (mark_used): Don't handle defer_mark_used_calls.

	* g++.dg/cpp0x/constexpr-69315.C: New test.
	* g++.dg/cpp0x/variadic122.C: Change one dg-warning into dg-bogus.

--- gcc/cp/cp-tree.h.jj	2016-03-21 10:12:31.000000000 +0100
+++ gcc/cp/cp-tree.h	2016-03-23 16:38:46.193501388 +0100
@@ -5846,8 +5846,6 @@ extern tree fndecl_declared_return_type
 extern bool undeduced_auto_decl			(tree);
 extern void require_deduced_type		(tree);
 
-extern bool defer_mark_used_calls;
-extern GTY(()) vec<tree, va_gc> *deferred_mark_used_calls;
 extern tree finish_case_label			(location_t, tree, tree);
 extern tree cxx_maybe_build_cleanup		(tree, tsubst_flags_t);
 
--- gcc/cp/decl.c.jj	2016-03-22 09:05:31.000000000 +0100
+++ gcc/cp/decl.c	2016-03-23 16:39:11.563158827 +0100
@@ -227,11 +227,6 @@ struct GTY((for_user)) named_label_entry
    function, two inside the body of a function in a local class, etc.)  */
 int function_depth;
 
-/* To avoid unwanted recursion, finish_function defers all mark_used calls
-   encountered during its execution until it finishes.  */
-bool defer_mark_used_calls;
-vec<tree, va_gc> *deferred_mark_used_calls;
-
 /* States indicating how grokdeclarator() should handle declspecs marked
    with __attribute__((deprecated)).  An object declared as
    __attribute__((deprecated)) suppresses warnings of uses of other
@@ -14594,9 +14589,6 @@ finish_function (int flags)
   if (c_dialect_objc ())
     objc_finish_function ();
 
-  gcc_assert (!defer_mark_used_calls);
-  defer_mark_used_calls = true;
-
   record_key_method_defined (fndecl);
 
   fntype = TREE_TYPE (fndecl);
@@ -14846,17 +14838,6 @@ finish_function (int flags)
   /* Clean up.  */
   current_function_decl = NULL_TREE;
 
-  defer_mark_used_calls = false;
-  if (deferred_mark_used_calls)
-    {
-      unsigned int i;
-      tree decl;
-
-      FOR_EACH_VEC_SAFE_ELT (deferred_mark_used_calls, i, decl)
-	mark_used (decl);
-      vec_free (deferred_mark_used_calls);
-    }
-
   invoke_plugin_callbacks (PLUGIN_FINISH_PARSE_FUNCTION, fndecl);
   return fndecl;
 }
--- gcc/cp/decl2.c.jj	2016-02-19 08:55:04.000000000 +0100
+++ gcc/cp/decl2.c	2016-03-23 16:39:28.604928715 +0100
@@ -5140,14 +5140,6 @@ mark_used (tree decl, tsubst_flags_t com
   if (DECL_ODR_USED (decl))
     return true;
 
-  /* If within finish_function, defer the rest until that function
-     finishes, otherwise it might recurse.  */
-  if (defer_mark_used_calls)
-    {
-      vec_safe_push (deferred_mark_used_calls, decl);
-      return true;
-    }
-
   /* Normally, we can wait until instantiation-time to synthesize DECL.
      However, if DECL is a static data member initialized with a constant
      or a constexpr function, we need it right now because a reference to
--- gcc/testsuite/g++.dg/cpp0x/constexpr-69315.C.jj	2016-03-23 16:48:59.675217676 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-69315.C	2016-03-23 16:46:37.000000000 +0100
@@ -0,0 +1,35 @@
+// PR c++/69315
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2" }
+
+// Template instantiation and evaluation for folding within
+// finish_function may call finish_function recursively.
+// Make sure we don't reject or delay that sort of recursion.
+
+template <bool> struct Iter;
+
+struct Arg {
+  Iter<true> begin();
+  Iter<true> end();
+};
+
+template <bool> struct Iter {
+  int operator*();
+  Iter operator++();
+  template <bool C1, bool C2> friend constexpr bool operator==(Iter<C1>, Iter<C2>);
+  template <bool C1, bool C2> friend constexpr bool operator!=(Iter<C1>, Iter<C2>);
+};
+
+void func(Arg a) {
+  for (auto ch : a) {
+    a.begin() == a.end();
+  }
+}
+
+template <bool C1, bool C2> constexpr bool operator==(Iter<C1>, Iter<C2>) {
+  return true;
+}
+
+template <bool C1, bool C2> constexpr bool operator!=(Iter<C1> a, Iter<C2> b) {
+  return a == b;
+}
--- gcc/testsuite/g++.dg/cpp0x/variadic122.C.jj	2014-03-10 10:50:13.000000000 +0100
+++ gcc/testsuite/g++.dg/cpp0x/variadic122.C	2016-03-23 16:54:10.000000000 +0100
@@ -9,7 +9,7 @@ template < class T > struct Container
 
 template < class T >
 T deref (T)
-{}				// { dg-warning "no return" }
+{}				// { dg-bogus "no return" }
 
 template < class T, class ... Args >
 auto deref (T u, int, Args ... args)->decltype (deref (u.f (), args ...))


	Jakub

Re: [PR69315] enable finish_function to recurse for constexpr functions

[Jason Merrill]

OK.

Jason