[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-03-29 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #29 from Martin Jambor  ---
(In reply to Jakub Jelinek from comment #28)
> So fixed for GCC 11 now?

Yes, it should be fixed in GCC 11.

We talked about backporting the patches to GCC 10 with Richi on IRC today and
decided to wait for potential fallout even if we miss the 10.3 release.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-03-29 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #28 from Jakub Jelinek  ---
So fixed for GCC 11 now?

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-03-24 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #27 from CVS Commits  ---
The master branch has been updated by Martin Jambor :

https://gcc.gnu.org/g:f225c6b0c50dc472e0b73b440b572a3bf1514020

commit r11-7817-gf225c6b0c50dc472e0b73b440b572a3bf1514020
Author: Martin Jambor 
Date:   Wed Mar 24 20:27:27 2021 +0100

ipa: Check that scalar types that IPA-CP comes up with are sane (PR99122)

This patch fixes the last bit of PR 99122 where various bits of IPA
infrastructure are presented with a program with type mismatches that
make it have undefined behavior, and when inlining or performing
IPA-CP, and encountering such mismatch, we basically try to
VIEW_CONVERT_EXPR whatever the caller has into whatever the callee has
or simply use an empty constructor if that cannot be done.  This
however does not work when the callee has VLA parameters because we
ICE in the process.

Richi has already disabled inlining for such cases, this patch avoids
the issue in IPA-CP.  It adds checks that whatever constant the
propagation arrived at is actually compatible or fold_convertible to
the callees formal parameer type.  Unlike in the past, we now have
types of all parameters of functions that we have analyzed, even with
LTO, and so can do it.

This should prevent only bogus propagations.  I have looked at the
effect of the patch on WPA of Firefox and did not have any.

I have bootstrapped and LTO bootstrapped and tested the patch on
x86_64-linux.  OK for trunk?  And perhaps later for GCC 10 too?

Thanks

gcc/ChangeLog:

2021-02-26  Martin Jambor  

PR ipa/99122
* ipa-cp.c (initialize_node_lattices): Mark as bottom all
parameters with unknown type.
(ipacp_value_safe_for_type): New function.
(propagate_vals_across_arith_jfunc): Verify that the constant type
can be used for a type of the formal parameter.
(propagate_vals_across_ancestor): Likewise.
(propagate_scalar_across_jump_function): Likewise.  Pass the type
also to propagate_vals_across_ancestor.

gcc/testsuite/ChangeLog:

2021-02-26  Martin Jambor  

PR ipa/99122
* gcc.dg/pr99122-3.c: Remove -fno-ipa-cp from options.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-03-06 Thread dcb314 at hotmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

David Binderman  changed:

   What|Removed |Added

 CC||dcb314 at hotmail dot com

--- Comment #26 from David Binderman  ---
Quite interestingly, this ice can also be made with C testsuite file
./gcc.dg/pr99122-3.c at -O2

It seems to have been iceing since before 20210214.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-03-05 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #25 from Martin Jambor  ---
I have proposed a patch for the IPA-CP part on the mailing list:
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566333.html

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-03-04 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #24 from Martin Jambor  ---
*** Bug 99194 has been marked as a duplicate of this bug. ***

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-25 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

Richard Biener  changed:

   What|Removed |Added

   Assignee|rguenth at gcc dot gnu.org |jamborm at gcc dot 
gnu.org

--- Comment #23 from Richard Biener  ---
Leaving the IPA-CP parts to Martin.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-19 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #22 from CVS Commits  ---
The master branch has been updated by Richard Biener :

https://gcc.gnu.org/g:1a2a7096e5e20d736c6138179470b21aa5a74864

commit r11-7296-g1a2a7096e5e20d736c6138179470b21aa5a74864
Author: Richard Biener 
Date:   Fri Feb 19 09:38:52 2021 +0100

middle-end/99122 - more VLA inlining fixes

This avoids declaring a function with VLA arguments or return values
as inlineable.  IPA CP still ICEs, so the testcase has that disabled.

2021-02-19  Richard Biener  

PR middle-end/99122
* tree-inline.c (inline_forbidden_p): Do not inline functions
with VLA arguments or return value.

* gcc.dg/pr99122-3.c: New testcase.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-19 Thread rguenther at suse dot de via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #21 from rguenther at suse dot de  ---
On Fri, 19 Feb 2021, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122
> 
> --- Comment #20 from Jakub Jelinek  ---
> I think you don't want variably_modified_type_p, that returns true even for
> pointers to VLAs (and references etc., perhaps many times indirectly).

Ah, indeed.  But then you want !poly_int_tree_p to not disallow
SVE vector types.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-19 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #20 from Jakub Jelinek  ---
I think you don't want variably_modified_type_p, that returns true even for
pointers to VLAs (and references etc., perhaps many times indirectly).
I think those should be fine, they are used fairly often unlike variable length
structures, and so we'd run into issues before.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-19 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #19 from Martin Jambor  ---
(In reply to Richard Biener from comment #17)
> there's variably_modified_type_p (you can pass NULL_TREE for the fndecl)
> which is more to the point.  Otherwise it looks reasonable.  Does IPA CP
> do things like IPA SRA and split aggregates?

No, it does not split them, the only SRAish thing it does is simple
removal of unused register-type parameters.

> I wonder in which cases IPA CP would derive "constants" for
> aggregates, so why are aggregate parameters even tracked?

I am not sure I understand the question.  The testcase in comment #12
attempts to pass a simple double constant to what is actually a VLA
structure, causing an ICE on undefined behavior input, which is the
main thing I want to avoid.

But I have just realized that if we now insist that we know all types
anyway - I have run the whole C testsuite and did not find any K
testcase where IPA-CP would consider a parameter for propagation when
not knowing its type - we can do something better and only propagate
when we know that force_value_to_type would not resort to building a
zero constructor.

This will allow to still propagate ("aggregate") constants within VLAs
like if there was one in b.n in the testcase (and the caller was
somewhat saner).

> I am testing the following for the inline issue for the last testcase,
> leaving the IPA CP one to you.

Sure, thanks.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-19 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #18 from Richard Biener  ---
Created attachment 50220
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50220=edit
patch

I am testing the following for the inline issue for the last testcase, leaving
the IPA CP one to you.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-19 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #17 from Richard Biener  ---
(In reply to Martin Jambor from comment #16)
> For the IPA-CP ICE, I am still running some tests, but I am currently
> leaning towards the following.  It might in theory disable IPA-CP in some
> strange K corner cases (I am searching for those with the tests), but I
> cannot say I care too much even if it does:
> 
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 167913cb927..4c8b76c09be 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1186,6 +1186,20 @@ set_single_call_flag (cgraph_node *node, void *)
>return false;
>  }
>  
> +/* Return true if the Ith formal parameter in function described by INFO
> has a
> +   type that is known and safe to accept constants.  */
> +
> +static bool
> +ipa_cp_param_has_safe_type_p (ipa_node_params *info, int i)
> +{
> +  tree t = ipa_get_type (info, i);
> +  if (!t)
> +return false;
> +  /* Attempting to propagate to parameters that are VLAs runs afoul
> limitations
> + of how clone materialization implementation.  */
> +  return TREE_CODE (TYPE_SIZE (t)) == INTEGER_CST;

there's variably_modified_type_p (you can pass NULL_TREE for the fndecl)
which is more to the point.  Otherwise it looks reasonable.  Does IPA CP
do things like IPA SRA and split aggregates?  I wonder in which cases
IPA CP would derive "constants" for aggregates, so why are aggregate
parameters even tracked?

> +}
> +
>  /* Initialize ipcp_lattices.  */
>  
>  static void
> @@ -1277,7 +1291,8 @@ initialize_node_lattices (struct cgraph_node *node)
>ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
>if (disable
>   || (pre_modified && (surviving_params.length () <= (unsigned) i
> -  || !surviving_params[i])))
> +  || !surviving_params[i]))
> + || !ipa_cp_param_has_safe_type_p (info, i))
> {
>   plats->itself.set_to_bottom ();
>   plats->ctxlat.set_to_bottom ();

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #16 from Martin Jambor  ---
For the IPA-CP ICE, I am still running some tests, but I am currently leaning
towards the following.  It might in theory disable IPA-CP in some strange K
corner cases (I am searching for those with the tests), but I cannot say I care
too much even if it does:

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 167913cb927..4c8b76c09be 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1186,6 +1186,20 @@ set_single_call_flag (cgraph_node *node, void *)
   return false;
 }

+/* Return true if the Ith formal parameter in function described by INFO has a
+   type that is known and safe to accept constants.  */
+
+static bool
+ipa_cp_param_has_safe_type_p (ipa_node_params *info, int i)
+{
+  tree t = ipa_get_type (info, i);
+  if (!t)
+return false;
+  /* Attempting to propagate to parameters that are VLAs runs afoul
limitations
+ of how clone materialization implementation.  */
+  return TREE_CODE (TYPE_SIZE (t)) == INTEGER_CST;
+}
+
 /* Initialize ipcp_lattices.  */

 static void
@@ -1277,7 +1291,8 @@ initialize_node_lattices (struct cgraph_node *node)
   ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
   if (disable
  || (pre_modified && (surviving_params.length () <= (unsigned) i
-  || !surviving_params[i])))
+  || !surviving_params[i]))
+ || !ipa_cp_param_has_safe_type_p (info, i))
{
  plats->itself.set_to_bottom ();
  plats->ctxlat.set_to_bottom ();

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #15 from CVS Commits  ---
The master branch has been updated by Richard Biener :

https://gcc.gnu.org/g:7ee164dcfe390fc030028db9112d44255637b1aa

commit r11-7278-g7ee164dcfe390fc030028db9112d44255637b1aa
Author: Richard Biener 
Date:   Thu Feb 18 12:28:26 2021 +0100

middle-end/99122 - Issues with VLA parameter inlining

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.

2021-02-18  Richard Biener  

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.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #14 from Martin Jambor  ---
Like with the following, which seems to work as far as inlining is concerned,
but the latest Jakub's example ICEs when cloning for IPA-CP :-/  (I am also not
sure if the predicate to identify VLAs is the best one).

--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3418,6 +3418,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 (TREE_CODE (value) == WITH_SIZE_EXPR)
+value = TREE_OPERAND (value, 0);
+  
   if (!is_gimple_reg_type (TREE_TYPE (value))
   || TYPE_SIZE (type) == TYPE_SIZE (TREE_TYPE (value)))
 return fold_build1 (VIEW_CONVERT_EXPR, type, value);
@@ -4017,6 +4020,10 @@ inline_forbidden_p (tree fndecl)
   wi.info = (void *) fndecl;
   wi.pset = _nodes;

+  for (tree parm = DECL_ARGUMENTS (fndecl); parm; parm = DECL_CHAIN (parm))
+if (TREE_CODE (DECL_SIZE (parm)) != INTEGER_CST)
+  return true;
+  
   FOR_EACH_BB_FN (bb, fun)
 { 
   gimple *ret;

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #13 from Martin Jambor  ---
I think that you want to disable inlining in the case when the callee has a
formal parameter which is a VLA (as opposed to a VLA actual argument of a
call), probably in inline_forbidden_p.  When just the caller does stupid
things, IMHO it can be fixed with the patch in comment #3.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #12 from Jakub Jelinek  ---
E.g.
static int foo ();

int
bar (int n)
{
  return foo (n, 2.0);
}

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;
}

ICEs at -O2 too.  And there is no WITH_SIZE_EXPR argument in that case.
So I think next to looking for WITH_SIZE_EXPR arguments it should also look
through callee DECL_ARGUMENTS if any argument has non-constant TYPE_SIZE* (or
DECL_SIZE*?).

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #11 from Jakub Jelinek  ---
And one case is not covered, callee like foo in #c2, but caller passing non-VLA
argument (whatever, a double, struct S { char a[4]; }, int, etc.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #10 from Richard Biener  ---
Like with

diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index e32e69cd3ad..ac85be741b1 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -2775,7 +2775,12 @@ analyze_function_body (struct cgraph_node *node, bool
early)
 (gimple_call_arg (stmt, i));
}
}
-
+ 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);

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #9 from Richard Biener  ---
(In reply to Jakub Jelinek from comment #2)
> Another, still undefined, but perhaps slightly less so, testcase is:
> 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;
> }
> 
> I wonder if the easiest fix isn't just to disable inlining if any of the
> arguments has variable length.  The inliner seems to be clearly unprepared
> to handle that.  VLAs when passed are passed as pointers to the elements, so
> it is only about variable length structures or if some front-end would pass
> arrays as arrays and not as pointers.

So with fixing the original and this case by simply not trying to be clever
when setting up the parameter on mismatches like with

@@ -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

the ICEs are gone but the comment#2 testcase has a local var added
which cannot be assembled because we do

  /* 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.  */
  var = copy_decl_to_var (p, id);

and obviously the sizes are not set up correctly for the caller.  That's
one complication - we have to massage the WITH_SIZE_EXPR size back to
the TYPE because the VLA bounds are not passed explicitely.  In a nested
function context those are transfered via the static chain so it should
work there but there the C FE marks the functions as not inlineable
(but IIRC for the unsupported return value handling).

Which makes me go back to IPA summary analysis, looking for the place
we can disable inlining of a certain cgraph edge based on call stmts
using WITH_SIZE_EXPR arguments.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-18 Thread rguenth at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

Richard Biener  changed:

   What|Removed |Added

   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org
 Status|NEW |ASSIGNED
   Priority|P3  |P2

--- Comment #8 from Richard Biener  ---
I pushed us in this direction, so let me have a look.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-17 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #7 from Jakub Jelinek  ---
(In reply to Martin Jambor from comment #6)
> > For punting on inlining these, I couldn't find any spot that would try to
> > verify at least remote compatibility of the passed in arguments and the
> > arguments the callee expects.
> 
> No, with LTO it would be too late even if we tried to, (IPA) inlining
> decisions are not meant to be un-doable.  The idea was that mismatches
> are undefined and so we should try our best to emulate non-inlining
> and not ICE.  But apparently we don't manage that now.

Well, I meant decide not to inline because either any of the gimple_call_arg
(stmt, ?) is a variable length or any of the expected arguments of the function
are variable length during the IPA inlining decisions.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-17 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #6 from Martin Jambor  ---
(In reply to Jakub Jelinek from comment #5)
> That could perhaps work for the #c0 testcase where the function actually has
> a non-VL parameter and so garbage in garbage out.
> But would that work also for #c2?

No, at least not on its own.  It leaks struct S, which is local to
bar, to the IL of foo, which hits an assert in make_decl_rtl.  Einline
dump has:

  struct T b;
  ...
  x.1_28 = __builtin_alloca_with_align (_26, 8);
  _20 = (long unsigned int) n_14(D);
  _21 = _28->a;
  __builtin_memset (_21, 0, _20);
  b = MEM  [(struct S *)x.1_28];

> If one dumps the #c2 testcase with -O2 -fno-inline -fdump-tree-optimized,
> the PARM_DECL is clearly variable length but not lowered to *ptr, while
> in the caller it is lowered that way and allocated through
> __builtin_alloca_with_align.
> So, clearly PARM_DECLs can be variable length but VAR_DECLs should not be
> (they should be gimplified into ptr = __builtin_alloca_with_align with stack
> save/restore around the scope and DECL_VALUE_EXPR of *ptr.
> The inliner certainly doesn't do that right now.
>
> For punting on inlining these, I couldn't find any spot that would try to
> verify at least remote compatibility of the passed in arguments and the
> arguments the callee expects.

No, with LTO it would be too late even if we tried to, (IPA) inlining
decisions are not meant to be un-doable.  The idea was that mismatches
are undefined and so we should try our best to emulate non-inlining
and not ICE.  But apparently we don't manage that now.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-17 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #5 from Jakub Jelinek  ---
That could perhaps work for the #c0 testcase where the function actually has a
non-VL parameter and so garbage in garbage out.
But would that work also for #c2?
If one dumps the #c2 testcase with -O2 -fno-inline -fdump-tree-optimized,
the PARM_DECL is clearly variable length but not lowered to *ptr, while
in the caller it is lowered that way and allocated through
__builtin_alloca_with_align.
So, clearly PARM_DECLs can be variable length but VAR_DECLs should not be (they
should be gimplified into ptr = __builtin_alloca_with_align with stack
save/restore around the scope and DECL_VALUE_EXPR of *ptr.
The inliner certainly doesn't do that right now.

For punting on inlining these, I couldn't find any spot that would try to
verify at least remote compatibility of the passed in arguments and the
arguments the callee expects.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-17 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #4 from Martin Jambor  ---
With the patch from comment #3, the following sequence with the problematic
call:

  x.1_26 = __builtin_alloca_with_align (_24, 8);
  g (WITH_SIZE_EXPR <*x.1_26, _22>, WITH_SIZE_EXPR <*x.1_26, _22>);
  __builtin_stack_restore (saved_stack.2_15);

is turned by einline into:

  x.1_26 = __builtin_alloca_with_align (_24, 8);
  x_27 = MEM  [(struct  *)x.1_26];
  y_29 = MEM  [(struct  *)x.1_26];
  _30 = x_27 u<= y_29;
  _31 = ~_30;
  _32 = (int) _31;
  __builtin_stack_restore (saved_stack.2_15);

Which looks OKish, under the circumstances, to me.  Let me look at the second
testcase now.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-17 Thread jamborm at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #3 from Martin Jambor  ---
Looking at how expr.c deals with WITH_SIZE_EXPR, perhaps we should do something
like the following:

diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index a710fa59027..cdabeb6bafd 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3418,6 +3418,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 (TREE_CODE (value) == WITH_SIZE_EXPR)
+value = TREE_OPERAND (value, 0);
+
   if (!is_gimple_reg_type (TREE_TYPE (value))
   || TYPE_SIZE (type) == TYPE_SIZE (TREE_TYPE (value)))
 return fold_build1 (VIEW_CONVERT_EXPR, type, value);

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-17 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

--- Comment #2 from Jakub Jelinek  ---
Another, still undefined, but perhaps slightly less so, testcase is:
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;
}

I wonder if the easiest fix isn't just to disable inlining if any of the
arguments has variable length.  The inliner seems to be clearly unprepared to
handle that.  VLAs when passed are passed as pointers to the elements, so it is
only about variable length structures or if some front-end would pass arrays as
arrays and not as pointers.

[Bug ipa/99122] [10/11 Regression] ICE in force_constant_size, at gimplify.c:733

2021-02-16 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99122

Jakub Jelinek  changed:

   What|Removed |Added

   Last reconfirmed||2021-02-16
 Ever confirmed|0   |1
 Status|UNCONFIRMED |NEW
  Component|c   |ipa
 CC||jakub at gcc dot gnu.org,
   ||jamborm at gcc dot gnu.org,
   ||marxin at gcc dot gnu.org
   Target Milestone|--- |10.3

--- Comment #1 from Jakub Jelinek  ---
Started with r10--g7313607478c11e9455a32fb0dbfd7867e04ea96a