tree-inline: Simplify IPA-CP type conversion (PR 93385)

Message ID ri6imhr91b6.fsf@suse.cz
State New
Headers show
Series
  • tree-inline: Simplify IPA-CP type conversion (PR 93385)
Related show

Commit Message

Martin Jambor April 22, 2020, 12:01 p.m.
Hi,

when callers and callees do not quite agree on the type of a
parameter, usually with ill-defined K&R or with smilarly wrong LTO
input, IPA-CP can attempt to try and substitute a wrong type for a
parameter (see e.g. gcc.dg/torture/pr48063.c).  Function
tree_function_versioning attempts to handle this in order not to
create invalid gimple but it then creates the mapping using
setup_one_parameter which also handles the same situation to avoid
similar problems when inlining and in more defined way.

So this patch simply removes the conversion attempts in
tree_function_versioning itself.  It is helpful for my upcoming fix of
PR 93385 because then I do not need to teach
ipa_param_body_adjustments to distinguish between successful and
unsuccessful mappings - setup_one_parameter uses force_value_to_type
for conversions which simly maps the worst cases to zero.

Bootstrapped, LTO-bootstrapped and tested on x86_64-linux.  I assume
this should wait until GCC 11 stage 1 but note that if we want to
backport a fix for PR 93385 later, this will probably be a prerequisite.

So, OK for trunk in stage 1?

Thanks,

Martin


2020-04-21  Martin Jambor  <mjambor@suse.cz>

	PR ipa/93385
	* tree-inline.c (tree_function_versioning): Leave any type conversion
	of replacements to setup_one_parameter and its friend
	force_value_to_type.
---
 gcc/ChangeLog     |  7 +++++++
 gcc/tree-inline.c | 42 +++++-------------------------------------
 2 files changed, 12 insertions(+), 37 deletions(-)

-- 
2.26.0

Comments

Richard Biener April 22, 2020, 12:22 p.m. | #1
On Wed, 22 Apr 2020, Martin Jambor wrote:

> Hi,

> 

> when callers and callees do not quite agree on the type of a

> parameter, usually with ill-defined K&R or with smilarly wrong LTO

> input, IPA-CP can attempt to try and substitute a wrong type for a

> parameter (see e.g. gcc.dg/torture/pr48063.c).  Function

> tree_function_versioning attempts to handle this in order not to

> create invalid gimple but it then creates the mapping using

> setup_one_parameter which also handles the same situation to avoid

> similar problems when inlining and in more defined way.

> 

> So this patch simply removes the conversion attempts in

> tree_function_versioning itself.  It is helpful for my upcoming fix of

> PR 93385 because then I do not need to teach

> ipa_param_body_adjustments to distinguish between successful and

> unsuccessful mappings - setup_one_parameter uses force_value_to_type

> for conversions which simly maps the worst cases to zero.

> 

> Bootstrapped, LTO-bootstrapped and tested on x86_64-linux.  I assume

> this should wait until GCC 11 stage 1 but note that if we want to

> backport a fix for PR 93385 later, this will probably be a prerequisite.

> 

> So, OK for trunk in stage 1?


OK for stage1.

Thanks,
Richard.

> Thanks,

> 

> Martin

> 

> 

> 2020-04-21  Martin Jambor  <mjambor@suse.cz>

> 

> 	PR ipa/93385

> 	* tree-inline.c (tree_function_versioning): Leave any type conversion

> 	of replacements to setup_one_parameter and its friend

> 	force_value_to_type.

> ---

>  gcc/ChangeLog     |  7 +++++++

>  gcc/tree-inline.c | 42 +++++-------------------------------------

>  2 files changed, 12 insertions(+), 37 deletions(-)

> 

> diff --git a/gcc/ChangeLog b/gcc/ChangeLog

> index 1edb5f2d70b..2578b6251af 100644

> --- a/gcc/ChangeLog

> +++ b/gcc/ChangeLog

> @@ -1,3 +1,10 @@

> +2020-04-21  Martin Jambor  <mjambor@suse.cz>

> +

> +	PR ipa/93385

> +	* tree-inline.c (tree_function_versioning): Leave any type conversion

> +	of replacements to setup_one_parameter and its friend

> +	force_value_to_type.

> +

>  2020-04-18  Jeff Law  <law@redhat.com>

>  

>  	PR debug/94439

> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c

> index 26c23f504be..fb1850382af 100644

> --- a/gcc/tree-inline.c

> +++ b/gcc/tree-inline.c

> @@ -6260,46 +6260,14 @@ tree_function_versioning (tree old_decl, tree new_decl,

>  	  p = new_param_indices[p];

>  

>  	tree parm;

> -	tree req_type, new_type;

> -

>  	for (parm = DECL_ARGUMENTS (old_decl); p;

>  	     parm = DECL_CHAIN (parm))

>  	  p--;

> -	tree old_tree = parm;

> -	req_type = TREE_TYPE (parm);

> -	new_type = TREE_TYPE (replace_info->new_tree);

> -	if (!useless_type_conversion_p (req_type, new_type))

> -	  {

> -	    if (fold_convertible_p (req_type, replace_info->new_tree))

> -	      replace_info->new_tree

> -		= fold_build1 (NOP_EXPR, req_type, replace_info->new_tree);

> -	    else if (TYPE_SIZE (req_type) == TYPE_SIZE (new_type))

> -	      replace_info->new_tree

> -		= fold_build1 (VIEW_CONVERT_EXPR, req_type,

> -			       replace_info->new_tree);

> -	    else

> -	      {

> -		if (dump_file)

> -		  {

> -		    fprintf (dump_file, "    const ");

> -		    print_generic_expr (dump_file,

> -					replace_info->new_tree);

> -		    fprintf (dump_file,

> -			     "  can't be converted to param ");

> -		    print_generic_expr (dump_file, parm);

> -		    fprintf (dump_file, "\n");

> -		  }

> -		old_tree = NULL;

> -	      }

> -	  }

> -

> -	if (old_tree)

> -	  {

> -	    init = setup_one_parameter (&id, old_tree, replace_info->new_tree,

> -					id.src_fn, NULL, &vars);

> -	    if (init)

> -	      init_stmts.safe_push (init);

> -	  }

> +	gcc_assert (parm);

> +	init = setup_one_parameter (&id, parm, replace_info->new_tree,

> +				    id.src_fn, NULL, &vars);

> +	if (init)

> +	  init_stmts.safe_push (init);

>        }

>  

>    ipa_param_body_adjustments *param_body_adjs = NULL;

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1edb5f2d70b..2578b6251af 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@ 
+2020-04-21  Martin Jambor  <mjambor@suse.cz>
+
+	PR ipa/93385
+	* tree-inline.c (tree_function_versioning): Leave any type conversion
+	of replacements to setup_one_parameter and its friend
+	force_value_to_type.
+
 2020-04-18  Jeff Law  <law@redhat.com>
 
 	PR debug/94439
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 26c23f504be..fb1850382af 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -6260,46 +6260,14 @@  tree_function_versioning (tree old_decl, tree new_decl,
 	  p = new_param_indices[p];
 
 	tree parm;
-	tree req_type, new_type;
-
 	for (parm = DECL_ARGUMENTS (old_decl); p;
 	     parm = DECL_CHAIN (parm))
 	  p--;
-	tree old_tree = parm;
-	req_type = TREE_TYPE (parm);
-	new_type = TREE_TYPE (replace_info->new_tree);
-	if (!useless_type_conversion_p (req_type, new_type))
-	  {
-	    if (fold_convertible_p (req_type, replace_info->new_tree))
-	      replace_info->new_tree
-		= fold_build1 (NOP_EXPR, req_type, replace_info->new_tree);
-	    else if (TYPE_SIZE (req_type) == TYPE_SIZE (new_type))
-	      replace_info->new_tree
-		= fold_build1 (VIEW_CONVERT_EXPR, req_type,
-			       replace_info->new_tree);
-	    else
-	      {
-		if (dump_file)
-		  {
-		    fprintf (dump_file, "    const ");
-		    print_generic_expr (dump_file,
-					replace_info->new_tree);
-		    fprintf (dump_file,
-			     "  can't be converted to param ");
-		    print_generic_expr (dump_file, parm);
-		    fprintf (dump_file, "\n");
-		  }
-		old_tree = NULL;
-	      }
-	  }
-
-	if (old_tree)
-	  {
-	    init = setup_one_parameter (&id, old_tree, replace_info->new_tree,
-					id.src_fn, NULL, &vars);
-	    if (init)
-	      init_stmts.safe_push (init);
-	  }
+	gcc_assert (parm);
+	init = setup_one_parameter (&id, parm, replace_info->new_tree,
+				    id.src_fn, NULL, &vars);
+	if (init)
+	  init_stmts.safe_push (init);
       }
 
   ipa_param_body_adjustments *param_body_adjs = NULL;