[PATCH] Do not fold anything during copy_fn (PR c++/90383)

[Richard Biener]

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  <jakub@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 <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)