| Message ID | BF230D13CA30DD48930C31D4099330003A458A6A@FMSMSX101.amr.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
Hi Iyer,
> First off, my sincerest apologies for letting this bug slip the cracks. I am attaching a patch that seem to work fine with the .i file that you have submitted in bugzilla for both C and C++. Please let me know if this fix works for you and if it is OK for trunk.
thanks for the patch. I've just bootstrapped it on i386-pc-solaris2.10
and all an-if.c failures are gone. That said, I wonder why we need the
separate pr57490.c testcase, which is practically a preprocessed version
of an-if.c with the HAVE_IO code removed.
I cannot approve the patch, though.
Rainer
> -----Original Message----- > From: Rainer Orth [mailto:ro@CeBiTec.Uni-Bielefeld.DE] > Sent: Tuesday, August 13, 2013 7:18 AM > To: Iyer, Balaji V > Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org; Marek Polacek > (polacek@redhat.com) > Subject: Re: [PATCH] Fix for PR c/57490 > > Hi Iyer, > > > First off, my sincerest apologies for letting this bug slip the cracks. I am > attaching a patch that seem to work fine with the .i file that you have submitted > in bugzilla for both C and C++. Please let me know if this fix works for you and if > it is OK for trunk. > > thanks for the patch. I've just bootstrapped it on i386-pc-solaris2.10 and all an- > if.c failures are gone. That said, I wonder why we need the separate pr57490.c > testcase, which is practically a preprocessed version of an-if.c with the HAVE_IO > code removed. Well, it has assert in it. A while back someone (I think it was Jason Merrill, but I could be wrong) told me to replace all asserts with builtin_abort. But the content inside the file is the same. > > I cannot approve the patch, though. > OK. Can I ask why? Is it because you do not have an authority or is it because you see some errors (e.g. formatting errors) in it? If it is the latter, can you tell me what the errors are so that I can go and fix them? > Rainer > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
Hi Iyer, >> thanks for the patch. I've just bootstrapped it on i386-pc-solaris2.10 >> and all an- >> if.c failures are gone. That said, I wonder why we need the separate >> pr57490.c >> testcase, which is practically a preprocessed version of an-if.c with the >> HAVE_IO >> code removed. > > Well, it has assert in it. A while back someone (I think it was Jason > Merrill, but I could be wrong) told me to replace all asserts with > builtin_abort. But the content inside the file is the same. in that case I'd suggest modifying an-if.c accordingly and not adding pr57490.c. >> I cannot approve the patch, though. > > OK. Can I ask why? Is it because you do not have an authority or is it > because you see some errors (e.g. formatting errors) in it? If it is the > latter, can you tell me what the errors are so that I can go and fix them? It's simply not my area of authority. Rainer
> -----Original Message----- > From: Rainer Orth [mailto:ro@CeBiTec.Uni-Bielefeld.DE] > Sent: Tuesday, August 13, 2013 9:38 AM > To: Iyer, Balaji V > Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org; Marek Polacek > (polacek@redhat.com) > Subject: Re: [PATCH] Fix for PR c/57490 > > Hi Iyer, > > >> thanks for the patch. I've just bootstrapped it on > >> i386-pc-solaris2.10 and all an- if.c failures are gone. That said, I > >> wonder why we need the separate pr57490.c testcase, which is > >> practically a preprocessed version of an-if.c with the HAVE_IO code > >> removed. > > > > Well, it has assert in it. A while back someone (I think it was Jason > > Merrill, but I could be wrong) told me to replace all asserts with > > builtin_abort. But the content inside the file is the same. > > in that case I'd suggest modifying an-if.c accordingly and not adding pr57490.c. > Well, on x86, an-if.c works fine without this modifications. It is just that in sparc the code is broken up a bit differently. > >> I cannot approve the patch, though. > > > > OK. Can I ask why? Is it because you do not have an authority or is it > > because you see some errors (e.g. formatting errors) in it? If it is > > the latter, can you tell me what the errors are so that I can go and fix them? > > It's simply not my area of authority. > Ok. Thanks for looking into this quickly though. Sincerely, Balaji V. Iyer. > Rainer > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
On 08/12/2013 01:16 PM, Iyer, Balaji V wrote: > + /* If it is a built-in array notation function, then the return type of > + the function is the type of the array passed in as array notation. */ How can the function return an array? Jason
> -----Original Message----- > From: Jason Merrill [mailto:jason@redhat.com] > Sent: Friday, August 16, 2013 2:08 PM > To: Iyer, Balaji V; Rainer Orth > Cc: Jakub Jelinek; gcc-patches@gcc.gnu.org; Marek Polacek > (polacek@redhat.com) > Subject: Re: [PATCH] Fix for PR c/57490 > > On 08/12/2013 01:16 PM, Iyer, Balaji V wrote: > > + /* If it is a built-in array notation function, then the return type of > > + the function is the type of the array passed in as array notation. */ > > How can the function return an array? > I guess I worded the comment poorly. Well, what I was going to say is this: float x, A[10]; x = __sec_reduce_add (A[:]); // The sec_reduce_add function's return type is the type of A[] which is float. int y, B[10]; y = __sec_reduce_add (B[:]); // ...the return type of __sec_reduce_add function is same as B[] which is int Thanks, Balaji V. Iyer. > Jason
On 08/16/2013 02:13 PM, Iyer, Balaji V wrote: >>> + /* If it is a built-in array notation function, then the return type of >>> + the function is the type of the array passed in as array notation. */ >> >> How can the function return an array? > > float x, A[10]; > x = __sec_reduce_add (A[:]); // The sec_reduce_add function's return type is the type of A[] which is float. Ah, then the comment should say "...is the element type of the array...". Jason
diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c index 7788f7b..5747bcb 100644 --- a/gcc/c/c-array-notation.c +++ b/gcc/c/c-array-notation.c @@ -906,6 +906,8 @@ fix_conditional_array_notations_1 (tree stmt) cond = COND_EXPR_COND (stmt); else if (TREE_CODE (stmt) == SWITCH_EXPR) cond = SWITCH_COND (stmt); + else if (truth_value_p (TREE_CODE (stmt))) + cond = TREE_OPERAND (stmt, 0); else /* Otherwise dont even touch the statement. */ return stmt; @@ -1232,6 +1234,12 @@ expand_array_notation_exprs (tree t) case BIND_EXPR: t = expand_array_notation_exprs (BIND_EXPR_BODY (t)); return t; + case TRUTH_ORIF_EXPR: + case TRUTH_ANDIF_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_XOR_EXPR: + case TRUTH_NOT_EXPR: case COND_EXPR: t = fix_conditional_array_notations (t); @@ -1246,8 +1254,6 @@ expand_array_notation_exprs (tree t) COND_EXPR_ELSE (t) = expand_array_notation_exprs (COND_EXPR_ELSE (t)); } - else - t = expand_array_notation_exprs (t); return t; case STATEMENT_LIST: { @@ -1284,6 +1290,10 @@ expand_array_notation_exprs (tree t) Replace those with just void zero node. */ t = void_zero_node; default: + for (int ii = 0; ii < TREE_CODE_LENGTH (TREE_CODE (t)); ii++) + if (contains_array_notation_expr (TREE_OPERAND (t, ii))) + TREE_OPERAND (t, ii) = + expand_array_notation_exprs (TREE_OPERAND (t, ii)); return t; } return t; diff --git a/gcc/cp/cp-array-notation.c b/gcc/cp/cp-array-notation.c index eb6a70d..f4581f0 100644 --- a/gcc/cp/cp-array-notation.c +++ b/gcc/cp/cp-array-notation.c @@ -857,6 +857,19 @@ cp_expand_cond_array_notations (tree orig_stmt) return error_mark_node; } } + else if (truth_value_p (TREE_CODE (orig_stmt))) + { + size_t left_rank = 0, right_rank = 0; + tree left_expr = TREE_OPERAND (orig_stmt, 0); + tree right_expr = TREE_OPERAND (orig_stmt, 1); + if (!find_rank (EXPR_LOCATION (left_expr), left_expr, left_expr, true, + &left_rank) + || !find_rank (EXPR_LOCATION (right_expr), right_expr, right_expr, + true, &right_rank)) + return error_mark_node; + if (right_rank == 0 && left_rank == 0) + return orig_stmt; + } if (!find_rank (EXPR_LOCATION (orig_stmt), orig_stmt, orig_stmt, true, &rank)) @@ -1213,6 +1226,12 @@ expand_array_notation_exprs (tree t) if (TREE_OPERAND (t, 0) == error_mark_node) return TREE_OPERAND (t, 0); return t; + case TRUTH_ANDIF_EXPR: + case TRUTH_ORIF_EXPR: + case TRUTH_AND_EXPR: + case TRUTH_OR_EXPR: + case TRUTH_XOR_EXPR: + case TRUTH_NOT_EXPR: case COND_EXPR: t = cp_expand_cond_array_notations (t); if (TREE_CODE (t) == COND_EXPR) @@ -1222,8 +1241,6 @@ expand_array_notation_exprs (tree t) COND_EXPR_ELSE (t) = expand_array_notation_exprs (COND_EXPR_ELSE (t)); } - else - t = expand_array_notation_exprs (t); return t; case FOR_STMT: if (contains_array_notation_expr (FOR_COND (t))) diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index e09c325..ba5286f 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3954,6 +3954,20 @@ cp_build_binary_op (location_t location, type0 = TREE_TYPE (op0); type1 = TREE_TYPE (op1); + /* If it is a built-in array notation function, then the return type of + the function is the type of the array passed in as array notation. */ + if (flag_enable_cilkplus && TREE_CODE (op1) == CALL_EXPR + && is_cilkplus_reduce_builtin (CALL_EXPR_FN (op1))) + { + tree array_ntn = CALL_EXPR_ARG (op1, 0); + type1 = TREE_TYPE (array_ntn); + } + if (flag_enable_cilkplus && TREE_CODE (op0) == CALL_EXPR + && is_cilkplus_reduce_builtin (CALL_EXPR_FN (op0))) + { + tree array_ntn = CALL_EXPR_ARG (op0, 0); + type0 = TREE_TYPE (array_ntn); + } /* The expression codes of the data types of the arguments tell us whether the arguments are integers, floating, pointers, etc. */ code0 = TREE_CODE (type0); diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57490.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57490.c new file mode 100644 index 0000000..db38b30 --- /dev/null +++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/pr57490.c @@ -0,0 +1,28 @@ +/* { dg-do run } */ +/* { dg-options "-fcilkplus" } */ + +const int n = 8; +float x[8], y[8], z[8]; +int main() { + int i = 0; + float x_sum =0; + for(i=1; i<=5; i+=4 ) { + x[0:n] = 3; + y[0:n] = i; + z[0:n] = 0; + (void)((__sec_reduce_add(x[0:n])==3*n) || (__builtin_abort (), 0)); + (void)((__sec_reduce_add(y[0:n])==i*n) || (__builtin_abort (), 0)); + (void)((__sec_reduce_add(z[0:n])==0) || (__builtin_abort (), 0)); + + if (x[0:n] >= y[0:n]) { + z[0:n] = x[0:n] - y[0:n]; + } else { + z[0:n] = x[0:n] + y[0:n]; + } + (void)((__sec_reduce_add(x[0:n])==3*n) || (__builtin_abort (), 0)); + (void)((__sec_reduce_add(y[0:n])==i*n) || (__builtin_abort (), 0)); + (void)((__sec_reduce_add(z[0:n])==(3>=i?3-i:3+i)*n) + || (__builtin_abort (), 0)); + } + return 0; +}