Re: [PATCH] Fix expansion ICE with call returning VL structure (PR middle-end/82564, PR target/88620)

2019-01-05 Thread Richard Biener
On January 3, 2019 11:55:16 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The following testcases ICE on x86_64 and other targets.
>
>The problem is that a function with nested functions returning VLA
>structures is inlined (or versioned) so that the variable sizes become
>constant, but we still have a MEM_REF[(struct S *) ] = fn ();
>where the D.1234 variable has fixed non-BLKmode size, but struct S is
>still a VLA type.  Furthermore, D.1234 isn't marked as addressable,
>because it is only accessed through such MEM_REFs.  Because of that
>mem_ref_refers_to_non_mem_p is true and we take a different
>expand_assignment path from normal VLA return expansion and try to
>create
>the VLA structure temporary, which ICEs obviously.
>
>The following patch fixes that by using a MEM temporary for this rare
>case.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK. 

Richard. 

>2019-01-03  Jakub Jelinek  
>
>   PR middle-end/82564
>   PR target/88620
>   * expr.c (expand_assignment): For calls returning VLA structures
>   if to_rtx is not a MEM, force it into a stack temporary.
>
>   * gcc.dg/nested-func-12.c: New test.
>   * gcc.c-torture/compile/pr82564.c: New test.
>
>--- gcc/expr.c.jj  2019-01-01 12:37:16.751981612 +0100
>+++ gcc/expr.c 2019-01-03 19:17:25.882346422 +0100
>@@ -5254,6 +5254,21 @@ expand_assignment (tree to, tree from, b
> emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp,
>true));
>   }
>   }
>+  /* For calls to functions returning variable length structures,
>if TO_RTX
>+   is not a MEM, go through a MEM because we must not create
>temporaries
>+   of the VLA type.  */
>+  else if (!MEM_P (to_rtx)
>+ && TREE_CODE (from) == CALL_EXPR
>+ && COMPLETE_TYPE_P (TREE_TYPE (from))
>+ && TREE_CODE (TYPE_SIZE (TREE_TYPE (from))) != INTEGER_CST)
>+  {
>+rtx temp = assign_stack_temp (GET_MODE (to_rtx),
>+  GET_MODE_SIZE (GET_MODE (to_rtx)));
>+result = store_field (temp, bitsize, bitpos, bitregion_start,
>+  bitregion_end, mode1, from, get_alias_set (to),
>+  nontemporal, reversep);
>+emit_move_insn (to_rtx, temp);
>+  }
>   else
>   {
> if (MEM_P (to_rtx))
>--- gcc/testsuite/gcc.dg/nested-func-12.c.jj   2019-01-03
>19:25:46.466084209 +0100
>+++ gcc/testsuite/gcc.dg/nested-func-12.c  2019-01-03 19:25:38.181220942
>+0100
>@@ -0,0 +1,48 @@
>+/* PR target/88620 */
>+/* { dg-do run } */
>+/* { dg-options "-Ofast --param ipa-cp-eval-threshold=0
>-fno-guess-branch-probability -fno-inline-small-functions" } */
>+/* { dg-require-effective-target alloca } */
>+
>+void
>+foo (int n)
>+{
>+  struct S { int a[n]; };
>+
>+  struct S
>+  fn (void)
>+  {
>+struct S s;
>+s.a[0] = 42;
>+return s;
>+  }
>+
>+  auto struct S
>+  fn2 (void)
>+  {
>+return fn ();
>+  }
>+
>+  struct S x;
>+  fn ();
>+  fn2 ();
>+  x = fn ();
>+
>+  if (x.a[0] != 42)
>+__builtin_abort ();
>+
>+  if (fn ().a[0] != 42)
>+__builtin_abort ();
>+
>+  __typeof__ (fn ()) *p = 
>+  if (p->a[0] != 42)
>+__builtin_abort ();
>+
>+  if (fn2 ().a[0] != 42)
>+__builtin_abort ();
>+}
>+
>+int
>+main (void)
>+{
>+  foo (1);
>+}
>--- gcc/testsuite/gcc.c-torture/compile/pr82564.c.jj   2019-01-03
>19:22:36.763215290 +0100
>+++ gcc/testsuite/gcc.c-torture/compile/pr82564.c  2019-01-03
>19:24:44.607105204 +0100
>@@ -0,0 +1,15 @@
>+/* PR middle-end/82564 */
>+/* { dg-require-effective-target alloca } */
>+
>+int
>+main ()
>+{
>+  int t = 8, i;
>+  typedef struct { char v[t]; } B; 
>+  B a, b;
>+  B __attribute__ ((noinline)) f () { return b; }
>+  for (i = 0; i < 8; i++)
>+b.v[i] = i;
>+  a = f ();
>+  return 0;
>+}
>
>   Jakub



[PATCH] Fix expansion ICE with call returning VL structure (PR middle-end/82564, PR target/88620)

2019-01-03 Thread Jakub Jelinek
Hi!

The following testcases ICE on x86_64 and other targets.

The problem is that a function with nested functions returning VLA
structures is inlined (or versioned) so that the variable sizes become
constant, but we still have a MEM_REF[(struct S *) ] = fn ();
where the D.1234 variable has fixed non-BLKmode size, but struct S is
still a VLA type.  Furthermore, D.1234 isn't marked as addressable,
because it is only accessed through such MEM_REFs.  Because of that
mem_ref_refers_to_non_mem_p is true and we take a different
expand_assignment path from normal VLA return expansion and try to create
the VLA structure temporary, which ICEs obviously.

The following patch fixes that by using a MEM temporary for this rare case.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-01-03  Jakub Jelinek  

PR middle-end/82564
PR target/88620
* expr.c (expand_assignment): For calls returning VLA structures
if to_rtx is not a MEM, force it into a stack temporary.

* gcc.dg/nested-func-12.c: New test.
* gcc.c-torture/compile/pr82564.c: New test.

--- gcc/expr.c.jj   2019-01-01 12:37:16.751981612 +0100
+++ gcc/expr.c  2019-01-03 19:17:25.882346422 +0100
@@ -5254,6 +5254,21 @@ expand_assignment (tree to, tree from, b
  emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp, true));
}
}
+  /* For calls to functions returning variable length structures, if TO_RTX
+is not a MEM, go through a MEM because we must not create temporaries
+of the VLA type.  */
+  else if (!MEM_P (to_rtx)
+  && TREE_CODE (from) == CALL_EXPR
+  && COMPLETE_TYPE_P (TREE_TYPE (from))
+  && TREE_CODE (TYPE_SIZE (TREE_TYPE (from))) != INTEGER_CST)
+   {
+ rtx temp = assign_stack_temp (GET_MODE (to_rtx),
+   GET_MODE_SIZE (GET_MODE (to_rtx)));
+ result = store_field (temp, bitsize, bitpos, bitregion_start,
+   bitregion_end, mode1, from, get_alias_set (to),
+   nontemporal, reversep);
+ emit_move_insn (to_rtx, temp);
+   }
   else
{
  if (MEM_P (to_rtx))
--- gcc/testsuite/gcc.dg/nested-func-12.c.jj2019-01-03 19:25:46.466084209 
+0100
+++ gcc/testsuite/gcc.dg/nested-func-12.c   2019-01-03 19:25:38.181220942 
+0100
@@ -0,0 +1,48 @@
+/* PR target/88620 */
+/* { dg-do run } */
+/* { dg-options "-Ofast --param ipa-cp-eval-threshold=0 
-fno-guess-branch-probability -fno-inline-small-functions" } */
+/* { dg-require-effective-target alloca } */
+
+void
+foo (int n)
+{
+  struct S { int a[n]; };
+
+  struct S
+  fn (void)
+  {
+struct S s;
+s.a[0] = 42;
+return s;
+  }
+
+  auto struct S
+  fn2 (void)
+  {
+return fn ();
+  }
+
+  struct S x;
+  fn ();
+  fn2 ();
+  x = fn ();
+
+  if (x.a[0] != 42)
+__builtin_abort ();
+
+  if (fn ().a[0] != 42)
+__builtin_abort ();
+
+  __typeof__ (fn ()) *p = 
+  if (p->a[0] != 42)
+__builtin_abort ();
+
+  if (fn2 ().a[0] != 42)
+__builtin_abort ();
+}
+
+int
+main (void)
+{
+  foo (1);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr82564.c.jj2019-01-03 
19:22:36.763215290 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr82564.c   2019-01-03 
19:24:44.607105204 +0100
@@ -0,0 +1,15 @@
+/* PR middle-end/82564 */
+/* { dg-require-effective-target alloca } */
+
+int
+main ()
+{
+  int t = 8, i;
+  typedef struct { char v[t]; } B; 
+  B a, b;
+  B __attribute__ ((noinline)) f () { return b; }
+  for (i = 0; i < 8; i++)
+b.v[i] = i;
+  a = f ();
+  return 0;
+}

Jakub