Fix factor_out_conditional_conversion caused ICE (PR tree-optimization/83521)

Message ID 20171221175045.GA2353@tucnak
State New
Headers show
Series
  • Fix factor_out_conditional_conversion caused ICE (PR tree-optimization/83521)
Related show

Commit Message

Jakub Jelinek Dec. 21, 2017, 5:50 p.m.
Hi!

The problem here is that the code expects fold_build1 will actually not
fold, because using gimple_build_assign (..., VIEW_CONVERT_EXPR, temp);
is valid only if TREE_CODE (temp) == VIEW_CONVERT_EXPR.
So, either we can replace the fold_build1 with build1, or we should just
use the gimple_build_assign overload that will handle even what
<view_convert_expr <SSA_NAME_result_of_phi>> will fold to (in this case
a <nop_expr <SSA_NAME_result_of_phi>>).

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

2017-12-21  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/83521
	* tree-ssa-phiopt.c (factor_out_conditional_conversion): Use
	gimple_build_assign without code on result of
	fold_build1 (VIEW_CONVERT_EXPR, ...), as it might not create
	a VIEW_CONVERT_EXPR.

	* gcc.dg/pr83521.c: New test.


	Jakub

Comments

Richard Biener Dec. 21, 2017, 6:56 p.m. | #1
On December 21, 2017 6:50:45 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!

>

>The problem here is that the code expects fold_build1 will actually not

>fold, because using gimple_build_assign (..., VIEW_CONVERT_EXPR, temp);

>is valid only if TREE_CODE (temp) == VIEW_CONVERT_EXPR.

>So, either we can replace the fold_build1 with build1, or we should

>just

>use the gimple_build_assign overload that will handle even what

><view_convert_expr <SSA_NAME_result_of_phi>> will fold to (in this case

>a <nop_expr <SSA_NAME_result_of_phi>>).

>

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


OK. 

Richard. 

>2017-12-21  Jakub Jelinek  <jakub@redhat.com>

>

>	PR tree-optimization/83521

>	* tree-ssa-phiopt.c (factor_out_conditional_conversion): Use

>	gimple_build_assign without code on result of

>	fold_build1 (VIEW_CONVERT_EXPR, ...), as it might not create

>	a VIEW_CONVERT_EXPR.

>

>	* gcc.dg/pr83521.c: New test.

>

>--- gcc/tree-ssa-phiopt.c.jj	2017-12-20 20:40:18.000000000 +0100

>+++ gcc/tree-ssa-phiopt.c	2017-12-21 11:25:33.335004838 +0100

>@@ -548,8 +548,12 @@ factor_out_conditional_conversion (edge

> 

>   /* Create the conversion stmt and insert it.  */

>   if (convert_code == VIEW_CONVERT_EXPR)

>-    temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp);

>-  new_stmt = gimple_build_assign (result, convert_code, temp);

>+    {

>+      temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result),

>temp);

>+      new_stmt = gimple_build_assign (result, temp);

>+    }

>+  else

>+    new_stmt = gimple_build_assign (result, convert_code, temp);

>   gsi = gsi_after_labels (gimple_bb (phi));

>   gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);

> 

>--- gcc/testsuite/gcc.dg/pr83521.c.jj	2017-12-21 11:29:54.348662949

>+0100

>+++ gcc/testsuite/gcc.dg/pr83521.c	2017-12-21 11:29:41.000000000 +0100

>@@ -0,0 +1,10 @@

>+/* PR tree-optimization/83521 */

>+/* { dg-do compile } */

>+/* { dg-options "-O1 -fno-tree-forwprop" } */

>+

>+int

>+foo (unsigned int x, int y)

>+{

>+  int *z = (int *)&x;

>+  return (y == 0) ? y : *z;

>+}

>

>	Jakub
Jeff Law Dec. 21, 2017, 11:14 p.m. | #2
On 12/21/2017 10:50 AM, Jakub Jelinek wrote:
> Hi!

> 

> The problem here is that the code expects fold_build1 will actually not

> fold, because using gimple_build_assign (..., VIEW_CONVERT_EXPR, temp);

> is valid only if TREE_CODE (temp) == VIEW_CONVERT_EXPR.

> So, either we can replace the fold_build1 with build1, or we should just

> use the gimple_build_assign overload that will handle even what

> <view_convert_expr <SSA_NAME_result_of_phi>> will fold to (in this case

> a <nop_expr <SSA_NAME_result_of_phi>>).

> 

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

> 

> 2017-12-21  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR tree-optimization/83521

> 	* tree-ssa-phiopt.c (factor_out_conditional_conversion): Use

> 	gimple_build_assign without code on result of

> 	fold_build1 (VIEW_CONVERT_EXPR, ...), as it might not create

> 	a VIEW_CONVERT_EXPR.

> 

> 	* gcc.dg/pr83521.c: New test.

OK.
jeff

Patch

--- gcc/tree-ssa-phiopt.c.jj	2017-12-20 20:40:18.000000000 +0100
+++ gcc/tree-ssa-phiopt.c	2017-12-21 11:25:33.335004838 +0100
@@ -548,8 +548,12 @@  factor_out_conditional_conversion (edge
 
   /* Create the conversion stmt and insert it.  */
   if (convert_code == VIEW_CONVERT_EXPR)
-    temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp);
-  new_stmt = gimple_build_assign (result, convert_code, temp);
+    {
+      temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp);
+      new_stmt = gimple_build_assign (result, temp);
+    }
+  else
+    new_stmt = gimple_build_assign (result, convert_code, temp);
   gsi = gsi_after_labels (gimple_bb (phi));
   gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
 
--- gcc/testsuite/gcc.dg/pr83521.c.jj	2017-12-21 11:29:54.348662949 +0100
+++ gcc/testsuite/gcc.dg/pr83521.c	2017-12-21 11:29:41.000000000 +0100
@@ -0,0 +1,10 @@ 
+/* PR tree-optimization/83521 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fno-tree-forwprop" } */
+
+int
+foo (unsigned int x, int y)
+{
+  int *z = (int *)&x;
+  return (y == 0) ? y : *z;
+}