diff mbox

[hsa,merge,04/10] Avoid extraneous remapping in copy_gimple_seq_and_replace_locals

Message ID 20160113173925.575530562@virgil.suse.cz
State New
Headers show

Commit Message

Martin Jambor Jan. 13, 2016, 5:39 p.m. UTC 
Hi,

this patch is new, it addresses a problem I outlined in
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00424.html and it is an
implementation of Jakub's suggestion in
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00614.html

I have refrained from bigger changes in struct copy_body_data in
tree-inline.h as I think that such a cleanup should be done
separately, but the structure could probably use some field-re
ordering to remove padding.

I hope I have grasped it correctly and that the patch is OK for trunk.

Thanks,

Martin


2016-01-13  Martin Jambor  <mjambor@suse.cz>

	* tree-inline.c (remap_decl): Use existing dclarations if
	remapping a type and prevent_decl_creation_for_types.
	(replace_locals_stmt): Do an initial remapping of non-VLA typed
	decls first.  Do real remapping with
	prevent_decl_creation_for_types set.
	* tree-inline.h (copy_body_data): New field
	prevent_decl_creation_for_types, moved remap_var_for_cilk to avoid
	padding.

Comments

Jakub Jelinek Jan. 13, 2016, 6:02 p.m. UTC | #1 
On Wed, Jan 13, 2016 at 06:39:29PM +0100, Martin Jambor wrote:
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -340,8 +340,22 @@ remap_decl (tree decl, copy_body_data *id)
>        return decl;
>      }
>  
> -  /* If we didn't already have an equivalent for this declaration,
> -     create one now.  */
> +  /* When remapping a type within copy_gimple_seq_and_replace_locals, all
> +     necessary DECLs have already been remapped and we do not want to duplicate
> +     a decl coming from outside of the sequence we are copying.  */
> +  if (!n
> +      && id->prevent_decl_creation_for_types
> +      && id->remapping_type_depth > 0
> +      && (VAR_P (decl) || TREE_CODE (decl) == PARM_DECL))
> +    {
> +      if (id->do_not_unshare)
> +	return decl;
> +      else
> +	return unshare_expr (decl);
> +    }

Just return decl; instead of the { if () return decl; else unshare_expr (decl); }
- both VAR_DECL and PARM_DECL are tcc_declaration for which unshare_expr
doesn't do anything.

Otherwise LGTM.  I'll get to your other patches tomorrow.

	Jakub
diff mbox

Patch

diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 6bf2467..7b34288 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -340,8 +340,22 @@  remap_decl (tree decl, copy_body_data *id)
       return decl;
     }
 
-  /* If we didn't already have an equivalent for this declaration,
-     create one now.  */
+  /* When remapping a type within copy_gimple_seq_and_replace_locals, all
+     necessary DECLs have already been remapped and we do not want to duplicate
+     a decl coming from outside of the sequence we are copying.  */
+  if (!n
+      && id->prevent_decl_creation_for_types
+      && id->remapping_type_depth > 0
+      && (VAR_P (decl) || TREE_CODE (decl) == PARM_DECL))
+    {
+      if (id->do_not_unshare)
+	return decl;
+      else
+	return unshare_expr (decl);
+    }
+
+  /* If we didn't already have an equivalent for this declaration, create one
+     now.  */
   if (!n)
     {
       /* Make a copy of the variable or label.  */
@@ -5225,8 +5239,19 @@  replace_locals_stmt (gimple_stmt_iterator *gsip,
       /* This will remap a lot of the same decls again, but this should be
 	 harmless.  */
       if (gimple_bind_vars (stmt))
-	gimple_bind_set_vars (stmt, remap_decls (gimple_bind_vars (stmt),
-						 NULL, id));
+	{
+	  tree old_var, decls = gimple_bind_vars (stmt);
+
+	  for (old_var = decls; old_var; old_var = DECL_CHAIN (old_var))
+	    if (!can_be_nonlocal (old_var, id)
+		&& ! variably_modified_type_p (TREE_TYPE (old_var), id->src_fn))
+	      remap_decl (old_var, id);
+
+	  gcc_checking_assert (!id->prevent_decl_creation_for_types);
+	  id->prevent_decl_creation_for_types = true;
+	  gimple_bind_set_vars (stmt, remap_decls (decls, NULL, id));
+	  id->prevent_decl_creation_for_types = false;
+	}
     }
 
   /* Keep iterating.  */
diff --git a/gcc/tree-inline.h b/gcc/tree-inline.h
index d3e5229..4cc1f19 100644
--- a/gcc/tree-inline.h
+++ b/gcc/tree-inline.h
@@ -140,14 +140,17 @@  struct copy_body_data
      the originals have been mapped to a value rather than to a
      variable.  */
   hash_map<tree, tree> *debug_map;
- 
-  /* Cilk keywords currently need to replace some variables that
-     ordinary nested functions do not.  */ 
-  bool remap_var_for_cilk;
 
   /* A map from the inlined functions dependence info cliques to
      equivalents in the function into which it is being inlined.  */
   hash_map<dependence_hash, unsigned short> *dependence_map;
+
+  /* Cilk keywords currently need to replace some variables that
+     ordinary nested functions do not.  */
+  bool remap_var_for_cilk;
+
+  /* Do not create new declarations when within type remapping.  */
+  bool prevent_decl_creation_for_types;
 };
 
 /* Weights of constructions for estimate_num_insns.  */