| Message ID | BF230D13CA30DD48930C31D4099330003A4BADE0@FMSMSX101.amr.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
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
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
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)))