middle-end/99122 - Issues with VLA parameter inlining

Message ID nycvar.YFH.7.76.2102181337010.11359@elmra.sevgm.obk
State New
Headers show
Series
  • middle-end/99122 - Issues with VLA parameter inlining
Related show

Commit Message

Richard Biener Feb. 18, 2021, 12:37 p.m.
The following instructs IPA not to inline calls with VLA parameters
and adjusts inlining not to create invalid view-converted VLA
parameters on mismatch and makes the error_mark paths with debug
stmts actually work.

The first part avoids the ICEs with the testcases already.

Bootstrapped and tested on x86_64-unknown-linux-gnu, does it
look sane?

Thanks,
Richard.

2021-02-18  Richard Biener  <rguenther@suse.de>

	PR middle-end/99122
	* ipa-fnsummary.c (analyze_function_body): Set
	CIF_FUNCTION_NOT_INLINABLE for VLA parameter calls.
	* tree-inline.c (insert_init_debug_bind): Pass NULL for
	error_mark_node values.
	(force_value_to_type): Do not build V_C_Es for WITH_SIZE_EXPR
	values.
	(setup_one_parameter): Delay force_value_to_type until when
	it's needed.

	* gcc.dg/pr99122-1.c: New testcase.
	* gcc.dg/pr99122-2.c: Likewise.
---
 gcc/ipa-fnsummary.c              |  8 +++++++-
 gcc/testsuite/gcc.dg/pr99122-1.c | 13 +++++++++++++
 gcc/testsuite/gcc.dg/pr99122-2.c | 21 +++++++++++++++++++++
 gcc/tree-inline.c                | 21 +++++++++++++--------
 4 files changed, 54 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr99122-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr99122-2.c

-- 
2.26.2

Comments

Paul Richard Thomas via Gcc-patches Feb. 18, 2021, 1:29 p.m. | #1
On Thu, Feb 18, 2021 at 01:37:29PM +0100, Richard Biener wrote:
> The following instructs IPA not to inline calls with VLA parameters

> and adjusts inlining not to create invalid view-converted VLA

> parameters on mismatch and makes the error_mark paths with debug

> stmts actually work.

> 

> The first part avoids the ICEs with the testcases already.

> 

> Bootstrapped and tested on x86_64-unknown-linux-gnu, does it

> look sane?

> 

> Thanks,

> Richard.

> 

> 2021-02-18  Richard Biener  <rguenther@suse.de>

> 

> 	PR middle-end/99122

> 	* ipa-fnsummary.c (analyze_function_body): Set

> 	CIF_FUNCTION_NOT_INLINABLE for VLA parameter calls.

> 	* tree-inline.c (insert_init_debug_bind): Pass NULL for

> 	error_mark_node values.

> 	(force_value_to_type): Do not build V_C_Es for WITH_SIZE_EXPR

> 	values.

> 	(setup_one_parameter): Delay force_value_to_type until when

> 	it's needed.

> 

> 	* gcc.dg/pr99122-1.c: New testcase.

> 	* gcc.dg/pr99122-2.c: Likewise.


LGTM, but for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122#c12
I guess we still need something like Martin's patch.
Of course it can be fixed incrementally.

	Jakub
Richard Biener Feb. 18, 2021, 1:39 p.m. | #2
On Thu, 18 Feb 2021, Jakub Jelinek wrote:

> On Thu, Feb 18, 2021 at 01:37:29PM +0100, Richard Biener wrote:

> > The following instructs IPA not to inline calls with VLA parameters

> > and adjusts inlining not to create invalid view-converted VLA

> > parameters on mismatch and makes the error_mark paths with debug

> > stmts actually work.

> > 

> > The first part avoids the ICEs with the testcases already.

> > 

> > Bootstrapped and tested on x86_64-unknown-linux-gnu, does it

> > look sane?

> > 

> > Thanks,

> > Richard.

> > 

> > 2021-02-18  Richard Biener  <rguenther@suse.de>

> > 

> > 	PR middle-end/99122

> > 	* ipa-fnsummary.c (analyze_function_body): Set

> > 	CIF_FUNCTION_NOT_INLINABLE for VLA parameter calls.

> > 	* tree-inline.c (insert_init_debug_bind): Pass NULL for

> > 	error_mark_node values.

> > 	(force_value_to_type): Do not build V_C_Es for WITH_SIZE_EXPR

> > 	values.

> > 	(setup_one_parameter): Delay force_value_to_type until when

> > 	it's needed.

> > 

> > 	* gcc.dg/pr99122-1.c: New testcase.

> > 	* gcc.dg/pr99122-2.c: Likewise.

> 

> LGTM, but for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122#c12

> I guess we still need something like Martin's patch.


Yeah.

> Of course it can be fixed incrementally.


Indeed, so I pushed the patch and will work on the followup, but
likely only tomorrow.

Richard.

Patch

diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index e32e69cd3ad..c3a25c52214 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -2775,7 +2775,13 @@  analyze_function_body (struct cgraph_node *node, bool early)
 			     (gimple_call_arg (stmt, i));
 		    }
 		}
-
+	      /* We cannot setup VLA parameters during inlining.  */
+	      for (unsigned int i = 0; i < gimple_call_num_args (stmt); ++i)
+		if (TREE_CODE (gimple_call_arg (stmt, i)) == WITH_SIZE_EXPR)
+		  {
+		    edge->inline_failed = CIF_FUNCTION_NOT_INLINABLE;
+		    break;
+		  }
 	      es->call_stmt_size = this_size;
 	      es->call_stmt_time = this_time;
 	      es->loop_depth = bb_loop_depth (bb);
diff --git a/gcc/testsuite/gcc.dg/pr99122-1.c b/gcc/testsuite/gcc.dg/pr99122-1.c
new file mode 100644
index 00000000000..5dfc0a85ad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr99122-1.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -w" } */
+
+void f ()
+{
+  int n = 5;
+  struct { char a[n]; } x;
+  g (x, x);
+}
+int g (double x, double y)
+{
+  return __builtin_isgreater (x, y);
+}
diff --git a/gcc/testsuite/gcc.dg/pr99122-2.c b/gcc/testsuite/gcc.dg/pr99122-2.c
new file mode 100644
index 00000000000..2b105428233
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr99122-2.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -w" } */
+
+static int foo ();
+
+int
+bar (int n)
+{
+  struct S { char a[n]; } x;
+  __builtin_memset (x.a, 0, n);
+  return foo (n, x);
+}
+
+static inline int
+foo (int n, struct T { char a[n]; } b)
+{
+  int r = 0, i;
+  for (i = 0; i < n; i++)
+    r += b.a[i];
+  return r;
+}
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index a710fa59027..01a08cf27be 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3352,7 +3352,10 @@  insert_init_debug_bind (copy_body_data *id,
 	base_stmt = gsi_stmt (gsi);
     }
 
-  note = gimple_build_debug_bind (tracked_var, unshare_expr (value), base_stmt);
+  note = gimple_build_debug_bind (tracked_var,
+				  value == error_mark_node
+				  ? NULL_TREE : unshare_expr (value),
+				  base_stmt);
 
   if (bb)
     {
@@ -3418,7 +3421,9 @@  force_value_to_type (tree type, tree value)
      Still if we end up with truly mismatched types here, fall back
      to using a VIEW_CONVERT_EXPR or a literal zero to not leak invalid
      GIMPLE to the following passes.  */
-  if (!is_gimple_reg_type (TREE_TYPE (value))
+  if (TREE_CODE (value) == WITH_SIZE_EXPR)
+    return error_mark_node;
+  else if (!is_gimple_reg_type (TREE_TYPE (value))
 	   || TYPE_SIZE (type) == TYPE_SIZE (TREE_TYPE (value)))
     return fold_build1 (VIEW_CONVERT_EXPR, type, value);
   else
@@ -3434,15 +3439,9 @@  setup_one_parameter (copy_body_data *id, tree p, tree value, tree fn,
 {
   gimple *init_stmt = NULL;
   tree var;
-  tree rhs = value;
   tree def = (gimple_in_ssa_p (cfun)
 	      ? ssa_default_def (id->src_cfun, p) : NULL);
 
-  if (value
-      && value != error_mark_node
-      && !useless_type_conversion_p (TREE_TYPE (p), TREE_TYPE (value)))
-    rhs = force_value_to_type (TREE_TYPE (p), value);
-
   /* Make an equivalent VAR_DECL.  Note that we must NOT remap the type
      here since the type of this decl must be visible to the calling
      function.  */
@@ -3501,6 +3500,12 @@  setup_one_parameter (copy_body_data *id, tree p, tree value, tree fn,
   if (TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (p)))
     TREE_READONLY (var) = 0;
 
+  tree rhs = value;
+  if (value
+      && value != error_mark_node
+      && !useless_type_conversion_p (TREE_TYPE (p), TREE_TYPE (value)))
+    rhs = force_value_to_type (TREE_TYPE (p), value);
+
   /* If there is no setup required and we are in SSA, take the easy route
      replacing all SSA names representing the function parameter by the
      SSA name passed to function.