It seems we need to unshare even non-CONSTRUCTOR expressions that we are
going to stick in the constexpr_call_table, so we don't end up sharing the
same e.g. ADDR_EXPR between two different functions.  I now think I
understand why unsharing CONSTRUCTOR arguments was improving memory
performance: separating the arguments from the caller function allows the
caller function to be GC'd better.  But it occurs to me that we don't need
to unshare until we decide that we're evaluating and caching this call, so
we can avoid the CONSTRUCTOR unshare/free pair for tentative arguments.
Freeing the tentative TREE_VEC still seems worth doing, so free_bindings
isn't going away entirely.

Tested x86_64-pc-linux-gnu, applying to trunk.

        * constexpr.c (cxx_bind_parameters_in_call): Don't unshare.
        (cxx_eval_call_expression): Unshare all args if we're caching.
---
 gcc/cp/constexpr.c                            | 32 +++++++++----------
 .../g++.dg/cpp0x/constexpr-string2.C          | 13 ++++++++
 2 files changed, 29 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-string2.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 19e09c74760..f3f03e7d621 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1441,9 +1441,6 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx, 
tree t,
 
       if (!*non_constant_p)
        {
-         /* Unsharing here isn't necessary for correctness, but it
-            significantly improves memory performance for some reason.  */
-         arg = unshare_constructor (arg);
          /* Make sure the binding has the same type as the parm.  But
             only for constant args.  */
          if (!TYPE_REF_P (type))
@@ -1959,19 +1956,11 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, 
tree t,
      this function exits.  */
   class free_bindings
   {
+    tree *bindings;
   public:
-    tree &bindings;
-    bool do_free;
-    free_bindings (tree &b): bindings (b), do_free(true) { }
-    void preserve () { do_free = false; }
-    ~free_bindings () {
-      if (do_free)
-       {
-         for (int i = 0; i < TREE_VEC_LENGTH (bindings); ++i)
-           free_constructor (TREE_VEC_ELT (bindings, i));
-         ggc_free (bindings);
-       }
-    }
+    free_bindings (tree &b): bindings (&b) { }
+    ~free_bindings () { if (bindings) ggc_free (*bindings); }
+    void preserve () { bindings = NULL; }
   } fb (new_call.bindings);
 
   if (*non_constant_p)
@@ -2074,7 +2063,18 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
          for (int i = 0; i < TREE_VEC_LENGTH (bound); ++i)
            {
              tree arg = TREE_VEC_ELT (bound, i);
-             /* Don't share a CONSTRUCTOR that might be changed.  */
+             if (entry)
+               {
+                 /* Unshare args going into the hash table to separate them
+                    from the caller's context, for better GC and to avoid
+                    problems with verify_gimple.  */
+                 arg = unshare_expr (arg);
+                 TREE_VEC_ELT (bound, i) = arg;
+               }
+             /* Don't share a CONSTRUCTOR that might be changed.  This is not
+                redundant with the unshare just above; we also don't want to
+                change the argument values in the hash table.  XXX Could we
+                unshare lazily in cxx_eval_store_expression?  */
              arg = unshare_constructor (arg);
              if (TREE_CODE (arg) == CONSTRUCTOR)
                vec_safe_push (ctors, arg);
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-string2.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-string2.C
new file mode 100644
index 00000000000..a64d815a7ba
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-string2.C
@@ -0,0 +1,13 @@
+// PR c++/91165
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -O }
+
+template <typename T> constexpr T bar (T c) { return c; }
+template <typename T, typename U> struct S {
+  T f;
+  U g;
+};
+template <typename T, typename U>
+constexpr S<T, U> foo (T &&c, U h) { return S<T, U> {c, bar (h)}; }
+void baz (int a) { foo (a, ""); }
+void qux () { foo (0, ""); }

base-commit: b82dc579862c8c67e7781af89be3391ad1cee28a
-- 
2.18.1

Reply via email to