On Fri, 10 May 2019, Jakub Jelinek wrote:
> Hi!
>
> The following testcases are rejects-valid or wrong-code, because
> when we make copies of the function for constexpr evaluation purposes (the
> primary intent is have the functions as is, with no folding whatsoever, so
> we diagnose everything), the inliner used under the hood to copy the function
> actually folds in some cases. This folding is done there to canonicalize
> some cases (MEM_REFs, INDIRECT_REFs, ADDR_EXRPs), but such canonicalization
> is only really needed if we replace their operands by something different
> (e.g. replace a PARM_DECL for the corresponding value etc.). When doing
> copy_fn, all we are changing is one set of decls for another set of decls
> of the same category.
>
> The following patch avoids those foldings during copy_fn.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> after a while to 9.2?
OK. Note in general canonicalization might be necessary if
we have any DECL_VALUE_EXPRs resolved.
Thanks,
Richard.
> 2019-05-10 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/90383
> * tree-inline.h (struct copy_body_data): Add do_not_fold member.
> * tree-inline.c (remap_gimple_op_r): Avoid folding expressions if
> id->do_not_fold.
> (copy_tree_body_r): Likewise.
> (copy_fn): Set id.do_not_fold to true.
>
> * g++.dg/cpp1y/constexpr-90383-1.C: New test.
> * g++.dg/cpp1y/constexpr-90383-2.C: New test.
>
> --- gcc/tree-inline.h.jj 2019-05-08 19:04:58.947797821 +0200
> +++ gcc/tree-inline.h 2019-05-09 10:18:43.567373908 +0200
> @@ -113,6 +113,9 @@ struct copy_body_data
> /* True if trees may not be unshared. */
> bool do_not_unshare;
>
> + /* True if trees should not be folded during the copying. */
> + bool do_not_fold;
> +
> /* True if new declarations may not be created during type remapping. */
> bool prevent_decl_creation_for_types;
>
> --- gcc/tree-inline.c.jj 2019-05-08 19:04:58.949797788 +0200
> +++ gcc/tree-inline.c 2019-05-09 10:41:49.691949208 +0200
> @@ -1101,7 +1101,7 @@ remap_gimple_op_r (tree *tp, int *walk_s
> /* Otherwise, just copy the node. Note that copy_tree_r already
> knows not to copy VAR_DECLs, etc., so this is safe. */
>
> - if (TREE_CODE (*tp) == MEM_REF)
> + if (TREE_CODE (*tp) == MEM_REF && !id->do_not_fold)
> {
> /* We need to re-canonicalize MEM_REFs from inline substitutions
> that can happen when a pointer argument is an ADDR_EXPR.
> @@ -1327,11 +1327,11 @@ copy_tree_body_r (tree *tp, int *walk_su
> tree type = TREE_TYPE (*tp);
> tree ptr = id->do_not_unshare ? *n : unshare_expr (*n);
> tree old = *tp;
> - *tp = gimple_fold_indirect_ref (ptr);
> + *tp = id->do_not_fold ? NULL : gimple_fold_indirect_ref (ptr);
> if (! *tp)
> {
> type = remap_type (type, id);
> - if (TREE_CODE (ptr) == ADDR_EXPR)
> + if (TREE_CODE (ptr) == ADDR_EXPR && !id->do_not_fold)
> {
> *tp
> = fold_indirect_ref_1 (EXPR_LOCATION (ptr), type, ptr);
> @@ -1360,7 +1360,7 @@ copy_tree_body_r (tree *tp, int *walk_su
> return NULL;
> }
> }
> - else if (TREE_CODE (*tp) == MEM_REF)
> + else if (TREE_CODE (*tp) == MEM_REF && !id->do_not_fold)
> {
> /* We need to re-canonicalize MEM_REFs from inline substitutions
> that can happen when a pointer argument is an ADDR_EXPR.
> @@ -1432,7 +1432,8 @@ copy_tree_body_r (tree *tp, int *walk_su
>
> /* Handle the case where we substituted an INDIRECT_REF
> into the operand of the ADDR_EXPR. */
> - if (TREE_CODE (TREE_OPERAND (*tp, 0)) == INDIRECT_REF)
> + if (TREE_CODE (TREE_OPERAND (*tp, 0)) == INDIRECT_REF
> + && !id->do_not_fold)
> {
> tree t = TREE_OPERAND (TREE_OPERAND (*tp, 0), 0);
> if (TREE_TYPE (t) != TREE_TYPE (*tp))
> @@ -6370,6 +6371,7 @@ copy_fn (tree fn, tree& parms, tree& res
> since front-end specific mechanisms may rely on sharing. */
> id.regimplify = false;
> id.do_not_unshare = true;
> + id.do_not_fold = true;
>
> /* We're not inside any EH region. */
> id.eh_lp_nr = 0;
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-90383-1.C.jj 2019-05-09
> 10:49:10.222509867 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-90383-1.C 2019-05-09
> 10:48:46.538910236 +0200
> @@ -0,0 +1,15 @@
> +// PR c++/90383
> +// { dg-do compile { target c++14 } }
> +
> +struct alignas(8) A { constexpr A (bool x) : a(x) {} A () = delete; bool a;
> };
> +struct B { A b; };
> +
> +constexpr bool
> +foo ()
> +{
> + B w{A (true)};
> + w.b = A (true);
> + return w.b.a;
> +}
> +
> +static_assert (foo (), "");
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-90383-2.C.jj 2019-05-09
> 10:49:18.194375099 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-90383-2.C 2019-05-09
> 10:51:08.433511507 +0200
> @@ -0,0 +1,22 @@
> +// PR c++/90383
> +// { dg-do run { target c++14 } }
> +// { dg-options "-O2" }
> +
> +extern "C" void abort ();
> +struct alignas(8) A { constexpr A (bool x) : a(x) {} A () = default; bool a;
> };
> +struct B { A b; };
> +
> +constexpr bool
> +foo ()
> +{
> + B w{A (true)};
> + w.b = A (true);
> + return w.b.a;
> +}
> +
> +int
> +main ()
> +{
> + if (!foo ())
> + abort ();
> +}
>
> Jakub
>
--
Richard Biener <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
