Patchwork fix for PR 59825

login
register
mail settings
Submitter Iyer, Balaji V
Date Jan. 15, 2014, 10:37 p.m.
Message ID <BF230D13CA30DD48930C31D4099330003A4BADE0@FMSMSX101.amr.corp.intel.com>
Download mbox | patch
Permalink /patch/311477/
State New
Headers show

Comments

Iyer, Balaji V - Jan. 15, 2014, 10:37 p.m.
Hello Everyone,
	Attached, please find a patch that will fix PR 59825. The main issue was array notations occurring in COMPOUND_EXPR. This patch should fix that and fix the rank_mismatch2.c test-case ICE.
	
	Ok for trunk?

Thanks,

Balaji V. Iyer.
Jakub Jelinek - Jan. 15, 2014, 10:54 p.m.
On Wed, Jan 15, 2014 at 10:37:04PM +0000, Iyer, Balaji V wrote:
> Hello Everyone,

> Attached, please find a patch that will fix PR 59825.  The main issue was
> array notations occurring in COMPOUND_EXPR.  This patch should fix that
> and fix the rank_mismatch2.c test-case ICE.

> --- a/gcc/c/c-array-notation.c
> +++ b/gcc/c/c-array-notation.c
> @@ -1289,6 +1289,15 @@ expand_array_notation_exprs (tree t)
>  	 A[x:y];
>  	 Replace those with just void zero node.  */
>        t = void_zero_node;
> +      return t;
> +    case COMPOUND_EXPR:
> +      if (contains_array_notation_expr (t))
> +	if (TREE_CODE (TREE_OPERAND (t, 0)) == SAVE_EXPR)
> +	  {
> +	    t = expand_array_notation_exprs (TREE_OPERAND (t, 1));
> +	    return t;
> +	  }
> +         /* Else fall through.  */
>      default:
>        for (int ii = 0; ii < TREE_CODE_LENGTH (TREE_CODE (t)); ii++)
>  	if (contains_array_notation_expr (TREE_OPERAND (t, ii)))

Why doesn't the default case handle it?  Furthermore, you are removing
the COMPOUND_EXPR and the SAVE_EXPR from the first operand of the
COMPOUND_EXPR, that reverts the effects of the fix if there are
array notations anywhere.

And last comment to the expand_array_notation_exprs, especially the C++ one,
wouldn't it be better to rewrite them as walk_tree/cp_walk_tree callbacks,
so that it really handles all expressions, not just a small subset of them?
E.g. in C++ you just don't look at all about OMP_PARALLEL etc., so I'd
expect you ICE if array notation is found inside of #pragma omp parallel
body.

	Jakub
Marek Polacek - Jan. 15, 2014, 10:59 p.m.
On Wed, Jan 15, 2014 at 10:37:04PM +0000, Iyer, Balaji V wrote:
> +2014-01-15  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	PR c/59825
> +	* c-array-notation.c (expand_array_notation_exprs): Added COMPOUND_EXPR
> +	case.
> +

"Add"

> --- a/gcc/c/c-array-notation.c
> +++ b/gcc/c/c-array-notation.c
> @@ -1289,6 +1289,15 @@ expand_array_notation_exprs (tree t)
>  	 A[x:y];
>  	 Replace those with just void zero node.  */
>        t = void_zero_node;
> +      return t;

Can't you just return void_zero_node; here?

> +    case COMPOUND_EXPR:
> +      if (contains_array_notation_expr (t))
> +	if (TREE_CODE (TREE_OPERAND (t, 0)) == SAVE_EXPR)
> +	  {
> +	    t = expand_array_notation_exprs (TREE_OPERAND (t, 1));
> +	    return t;

And return expand_array_notation_exprs (TREE_OPERAND (t, 1)); here?
Also, is that if (contains_array_notation_expr (t)) needed?  At the
start of the function there's 

1229   if (!contains_array_notation_expr (t))
1230     return t;

	Marek

Patch

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 4754bdf..e081c19 100644
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-01-15  Balaji V. Iyer  <balaji.v.iyer@intel.com>
+
+	PR c/59825
+	* c-array-notation.c (expand_array_notation_exprs): Added COMPOUND_EXPR
+	case.
+
 2014-01-15  Jakub Jelinek  <jakub@redhat.com>
 
 	PR c/58943
diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
index 5526ee9..dee9fa9 100644
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -1289,6 +1289,15 @@  expand_array_notation_exprs (tree t)
 	 A[x:y];
 	 Replace those with just void zero node.  */
       t = void_zero_node;
+      return t;
+    case COMPOUND_EXPR:
+      if (contains_array_notation_expr (t))
+	if (TREE_CODE (TREE_OPERAND (t, 0)) == SAVE_EXPR)
+	  {
+	    t = expand_array_notation_exprs (TREE_OPERAND (t, 1));
+	    return t;
+	  }
+         /* Else fall through.  */
     default:
       for (int ii = 0; ii < TREE_CODE_LENGTH (TREE_CODE (t)); ii++)
 	if (contains_array_notation_expr (TREE_OPERAND (t, ii)))