diff mbox

Fix for PR c/57490

Message ID BF230D13CA30DD48930C31D4099330003A45A0C6@FMSMSX101.amr.corp.intel.com
State New
Headers show

Commit Message

Iyer, Balaji V Aug. 18, 2013, 11:42 p.m. UTC
> -----Original Message-----
> From: Jason Merrill [mailto:jason@redhat.com]
> Sent: Saturday, August 17, 2013 12:55 AM
> 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/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...".

Thanks. Here is a fixed patch. Is this Ok for trunk?

Here are the ChangeLog entries
gcc/c/ChangeLog
2013-08-18  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        PR c/57490
        * c-array-notation.c (fix_conditional_array_notations_1): Added a
        check for truth values.
        (expand_array_notation_exprs): Added truth values case.  Removed an
        unwanted else.  Added for-loop to walk through subtrees in default
        case.

gcc/cp/ChangeLog
2013-08-18  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        PR c/57490
        * cp-array-notation.c (cp_expand_cond_array_notations): Added a
        check for truth values.
        (expand_array_notation_exprs): Added truth values case.  Removed an
        unwanted else.  Added for-loop to walk through subtrees in default
        case.
        * typeck.c (cp_build_binary_op): Inherited the type of the array
        notation for built-in array notation functions.

gcc/testsuite/ChangeLog
2013-08-18  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        PR c/57490
        * c-c++-common/cilk-plus/AN/pr57490.c: New test.

sincerely,

Balaji V. Iyer.

> 
> Jason

Comments

Jason Merrill Aug. 19, 2013, 1:51 p.m. UTC | #1
On 08/18/2013 07:42 PM, Iyer, Balaji V wrote:
>> 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.  */
>>>>
>> Ah, then the comment should say "...is the element type of the array...".

Hmm, now it occurs to me to wonder why the CALL_EXPR doesn't already 
have the appropriate type?

Jason
Iyer, Balaji V Aug. 19, 2013, 2:06 p.m. UTC | #2
> -----Original Message-----
> From: Jason Merrill [mailto:jason@redhat.com]
> Sent: Monday, August 19, 2013 9:51 AM
> 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/18/2013 07:42 PM, Iyer, Balaji V wrote:
> >> 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.  */
> >>>>
> >> Ah, then the comment should say "...is the element type of the array...".
> 
> Hmm, now it occurs to me to wonder why the CALL_EXPR doesn't already have
> the appropriate type?
> 

Well, it is described in cilkplus.def. The return type of it changes based on the array that is passed in. So, it is given a fake type. Thus, we need to fix it up here.


> Jason
>
Jason Merrill Aug. 19, 2013, 2:19 p.m. UTC | #3
On 08/19/2013 10:06 AM, Iyer, Balaji V wrote:
> Well, it is described in cilkplus.def. The return type of it changes based on the array that is passed in. So, it is given a fake type. Thus, we need to fix it up here.

Right, but it should be fixed up when the CALL_EXPR is created, rather 
than when that CALL_EXPR is used in another expression.

Jason
Iyer, Balaji V Aug. 19, 2013, 2:37 p.m. UTC | #4
> -----Original Message-----
> From: Jason Merrill [mailto:jason@redhat.com]
> Sent: Monday, August 19, 2013 10:20 AM
> 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/19/2013 10:06 AM, Iyer, Balaji V wrote:
> > Well, it is described in cilkplus.def. The return type of it changes based on the
> array that is passed in. So, it is given a fake type. Thus, we need to fix it up here.
> 
> Right, but it should be fixed up when the CALL_EXPR is created, rather than
> when that CALL_EXPR is used in another expression.
> 

HI Jason,
	I just want to make sure I get what you are saying. Are you suggesting that I do this in finish_call_expr() instead of cp_build_binary_op() ?

Thanks,

Balaji V. Iyer.

> Jason
Jason Merrill Aug. 19, 2013, 2:45 p.m. UTC | #5
On 08/19/2013 10:37 AM, Iyer, Balaji V wrote:
> 	I just want to make sure I get what you are saying. Are you suggesting that I do this in finish_call_expr() instead of cp_build_binary_op() ?

I think build_cxx_call is the right place.

Jason
diff mbox

Patch

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..b33eb58 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3954,6 +3954,21 @@  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 element type of the array passed in as array 
+     notation (i.e. the first parameter of the function).  */
+  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;
+}