On 7/30/20 9:49 AM, Patrick Palka wrote:
On Thu, 30 Jul 2020, Jason Merrill wrote:

On 7/21/20 3:07 PM, Patrick Palka wrote:
In the testcase from the PR we are seeing excessive memory use (> 5GB)
during constexpr evaluation, almost all of which is due to the call to
decl_constant_value in the VAR_DECL/CONST_DECL branch of
cxx_eval_constant_expression.  We reach here every time we evaluate an
ARRAY_REF of a constexpr VAR_DECL, which in this testcase is quite
often, and from there decl_constant_value makes an unshared copy of the
VAR_DECL's initializer, even though the unsharing is not needed at this
call site (because it is up to callers of cxx_eval_constant_expression
to unshare).

To fix this, this patch moves the responsibility of unsharing the result
of decl_constant_value, decl_really_constant_value and
scalar_constant_value from the callee to the caller.

How about creating another entry point that doesn't unshare, and using that in
constexpr evaluation?

Is something like this what you have in mind?  This adds a defaulted
bool parameter to the three routines that controls unsharing (except for
decl_constant_value, which instead needs a new overload if we don't want
to touch its common declaration in c-common.h.)  Bootstrap and regtest
in progress.

That looks good, though I don't think we need the added parameter for scalar_constant_value.

-- >8 --

Subject: [PATCH] c++: decl_constant_value and unsharing [PR96197]

In the testcase from the PR we are seeing excessive memory use (> 5GB)
during constexpr evaluation, almost all of which is due to the call to
decl_constant_value in the VAR_DECL/CONST_DECL branch of
cxx_eval_constant_expression.  We reach here every time we evaluate an
ARRAY_REF of a constexpr VAR_DECL, which in this testcase is quite
often, and from there decl_constant_value makes an unshared copy of the
VAR_DECL's initializer, even though the unsharing is not needed at this
call site (because callers of cxx_eval_constant_expression already
unshare its result when necessary).

To fix this excessive unsharing, this patch introduces a new defaulted
parameter unshare_p to scalar_constant_value, decl_really_constant_value
and decl_constant_value to allow callers to decide if these routines
should unshare their result before returning.  (Since decl_constant_value
is declared in c-common.h, it instead gets a new overload declared in
cp-tree.h.)

As a simplification, this patch also moves the call to unshare_expr in
constant_value_1 outside of the loop, since calling unshare_expr on a
DECL_P should be a no-op.

Additionally, in unify there is one call to scalar_constant_value that
seems to be dead code since we apparently don't see unlowered
enumerators anymore, so this patch replaces it with an appropriate
gcc_assert.

Now that the the calls to decl_constant_value and
decl_really_constant_value from cxx_eval_constant_expression no longer
unshare their result, memory use during constexpr evaluation for the
testcase from the PR falls from ~5GB to 15MB according to -ftime-report.

gcc/cp/ChangeLog:

        PR c++/96197
        * constexpr.c (cxx_eval_constant_expression) <case CONST_DECL>:
        Pass false as the decl_constant_value and
        decl_really_constant_value so that they don't unshare their
        result.
        * cp-tree.h (decl_constant_value): New declaration with an
        additional bool parameter.
        (scalar_constant_value): Add defaulted bool parameter.
        (decl_really_constant_value): Likewise.
        * cp-gimplify.c (cp_fold_maybe_rvalue): Call unshare_expr on the
        result of decl_constant_value.
        * init.c (constant_value_1): Add bool parameter.  Conditionally
        unshare the initializer only before returning.
        (scalar_constant_value): Add bool parameter and pass it to
        constant_value_1
        (decl_really_constant_value): Likewise.
        (decl_constant_value): New overload with an additional bool
        parameter.
        * pt.c (tsubst_copy) <case CONST_DECL>: Assert
        DECL_TEMPLATE_PARM_P and simplify accordingly.

gcc/testsuite/ChangeLog:

        PR c++/96197
        * g++.dg/cpp1y/constexpr-array8.C: New test.
---
  gcc/cp/constexpr.c                            |  4 +--
  gcc/cp/cp-tree.h                              |  5 +--
  gcc/cp/init.c                                 | 35 ++++++++++++-------
  gcc/cp/pt.c                                   |  7 ++--
  gcc/testsuite/g++.dg/cpp1y/constexpr-array8.C | 18 ++++++++++
  5 files changed, 48 insertions(+), 21 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-array8.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 97dcc1b1d10..b1c1d249c6e 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5695,9 +5695,9 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
          TREE_CONSTANT (r) = true;
        }
        else if (ctx->strict)
-       r = decl_really_constant_value (t);
+       r = decl_really_constant_value (t, /*unshare_p=*/false);
        else
-       r = decl_constant_value (t);
+       r = decl_constant_value (t, /*unshare_p=*/false);
        if (TREE_CODE (r) == TARGET_EXPR
          && TREE_CODE (TARGET_EXPR_INITIAL (r)) == CONSTRUCTOR)
        r = TARGET_EXPR_INITIAL (r);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ea4871f836a..e32e5bc8eb2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6772,8 +6772,9 @@ extern tree build_vec_delete                      
(location_t, tree, tree,
                                                 tsubst_flags_t);
  extern tree create_temporary_var              (tree);
  extern void initialize_vtbl_ptrs              (tree);
-extern tree scalar_constant_value              (tree);
-extern tree decl_really_constant_value         (tree);
+extern tree decl_constant_value                        (tree, bool);
+extern tree scalar_constant_value              (tree, bool = true);
+extern tree decl_really_constant_value         (tree, bool = true);
  extern int diagnose_uninitialized_cst_or_ref_member (tree, bool, bool);
  extern tree build_vtbl_address                  (tree);
  extern bool maybe_reject_flexarray_init               (tree, tree);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 913fa4a0080..8298ec17fde 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2277,10 +2277,12 @@ build_offset_ref (tree type, tree member, bool 
address_p,
     recursively); otherwise, return DECL.  If STRICT_P, the
     initializer is only returned if DECL is a
     constant-expression.  If RETURN_AGGREGATE_CST_OK_P, it is ok to
-   return an aggregate constant.  */
+   return an aggregate constant.  The returned initializer is unshared
+   iff UNSHARE_P. */
static tree
-constant_value_1 (tree decl, bool strict_p, bool return_aggregate_cst_ok_p)
+constant_value_1 (tree decl, bool strict_p, bool return_aggregate_cst_ok_p,
+                 bool unshare_p)
  {
    while (TREE_CODE (decl) == CONST_DECL
         || decl_constant_var_p (decl)
@@ -2348,40 +2350,49 @@ constant_value_1 (tree decl, bool strict_p, bool 
return_aggregate_cst_ok_p)
          && !DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
          && DECL_NONTRIVIALLY_INITIALIZED_P (decl))
        break;
-      decl = unshare_expr (init);
+      decl = init;
      }
-  return decl;
+  return unshare_p ? unshare_expr (decl) : decl;
  }
/* If DECL is a CONST_DECL, or a constant VAR_DECL initialized by constant
     of integral or enumeration type, or a constexpr variable of scalar type,
     then return that value.  These are those variables permitted in constant
-   expressions by [5.19/1].  */
+   expressions by [5.19/1].  The returned value is unshared iff UNSHARE_P.  */
tree
-scalar_constant_value (tree decl)
+scalar_constant_value (tree decl, bool unshare_p /*= true*/)
  {
    return constant_value_1 (decl, /*strict_p=*/true,
-                          /*return_aggregate_cst_ok_p=*/false);
+                          /*return_aggregate_cst_ok_p=*/false,
+                          /*unshare_p=*/unshare_p);
  }
/* Like scalar_constant_value, but can also return aggregate initializers. */ tree
-decl_really_constant_value (tree decl)
+decl_really_constant_value (tree decl, bool unshare_p /*= true*/)
  {
    return constant_value_1 (decl, /*strict_p=*/true,
-                          /*return_aggregate_cst_ok_p=*/true);
+                          /*return_aggregate_cst_ok_p=*/true,
+                          /*unshare_p=*/unshare_p);
  }
-/* A more relaxed version of scalar_constant_value, used by the
+/* A more relaxed version of decl_really_constant_value, used by the
     common C/C++ code.  */
tree
-decl_constant_value (tree decl)
+decl_constant_value (tree decl, bool unshare_p)
  {
    return constant_value_1 (decl, /*strict_p=*/processing_template_decl,
-                          /*return_aggregate_cst_ok_p=*/true);
+                          /*return_aggregate_cst_ok_p=*/true,
+                          /*unshare_p=*/unshare_p);
+}
+
+tree
+decl_constant_value (tree decl)
+{
+  return decl_constant_value (decl, /*unshare_p=*/true);
  }
  
  /* Common subroutines of build_new and build_vec_delete.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 72bdf869b55..3a168d506cf 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -23664,11 +23664,8 @@ unify (tree tparms, tree targs, tree parm, tree arg, 
int strict,
                    strict, explain_p);
case CONST_DECL:
-      if (DECL_TEMPLATE_PARM_P (parm))
-       return unify (tparms, targs, DECL_INITIAL (parm), arg, strict, 
explain_p);
-      if (arg != scalar_constant_value (parm))
-       return unify_template_argument_mismatch (explain_p, parm, arg);
-      return unify_success (explain_p);
+      gcc_assert (DECL_TEMPLATE_PARM_P (parm));
+      return unify (tparms, targs, DECL_INITIAL (parm), arg, strict, 
explain_p);
case FIELD_DECL:
      case TEMPLATE_DECL:
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-array8.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-array8.C
new file mode 100644
index 00000000000..339abb69019
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-array8.C
@@ -0,0 +1,18 @@
+// PR c++/96197
+// { dg-do compile { target c++14 } }
+
+struct S {
+  S* p = this;
+};
+
+constexpr S ary[5000] = {};
+
+constexpr int foo() {
+  int count = 0;
+  for (int i = 0; i < 5000; i++)
+    if (ary[i].p != nullptr)
+      count++;
+  return count;
+}
+
+constexpr int bar = foo();


Reply via email to