public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix for PR c/57563
@ 2013-06-09  1:20 Iyer, Balaji V
  2013-06-10 14:39 ` Joseph S. Myers
  0 siblings, 1 reply; 9+ messages in thread
From: Iyer, Balaji V @ 2013-06-09  1:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek, mpolacek

[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]

Hello Everyone,
	Attached, please find a patch that will fix the bug reported in PR 57563. There are a couple issues that went wrong. First, in the test case, we have a double multiplied to a double. When -std=c99 flag is used, they get converted to long double. The way to fix this is to add a type cast to the array notation to the same type as identity variable and thus they will all be double. 
	The second issue, was that a sec_reduce_mutating function takes in the address of a "mutating variable" (i.e. the variable that will hold the result), the array notation and a function pointer. For example, for the following code:

int a[10], x = 0;
void function_name (int *p, int r);
__sec_reduce_mutating (&x,  a[0:10], function_name);


__sec_reduce_mutating should be converted to:

for (ii =0; ii < 10; ii++)
	function_name (&x, a[ii]);

	In the test case I was not representing this correctly (as shown in the conversion above), but just computing the value that the function should do, thus making the test flaky. I made this fix in the test case. The other advantage of this change is that, in future I can change the what the function does (maybe with #defines and have multiple checks for different function body) and I don't have to change a lot of things.

I tried the patch on x86 and x86_64 and it works fine. I am assuming -m32 on x86_64 should have the same behavior as x86. So, is this OK for trunk?

Here are the Changelog entries:

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

        * c-array-notation.c (fix_builtin_array_notation_fn): Added a cast
        for all the usage of function parameter to match the identity var.

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

        PR c/57563
        * c-c++-common/cilk-plus/AN/builtin_fn_mutating.c (main): Fixed a bug
        in how we check __sec_reduce_mutating function's result.

Thanks,

Balaji V. Iyer.

[-- Attachment #2: patch_pr57563.txt --]
[-- Type: text/plain, Size: 5902 bytes --]

diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
index 5fbb31f..caf2146 100644
Binary files a/gcc/c/ChangeLog and b/gcc/c/ChangeLog differ
diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
index b1040da..1914a24 100644
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -133,7 +133,7 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
   bool **count_down, **array_vector;
   location_t location = UNKNOWN_LOCATION;
   tree loop_with_init = alloc_stmt_list ();
-  
+  tree new_comp_expr = NULL_TREE, identity_expr = NULL_TREE;
   enum built_in_function an_type =
     is_cilkplus_reduce_builtin (CALL_EXPR_FN (an_builtin_fn));
   if (an_type == BUILT_IN_NONE)
@@ -483,10 +483,12 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
       new_yes_expr = build_modify_expr
 	(location, *new_var, TREE_TYPE (*new_var), NOP_EXPR,
 	 location, func_parm, TREE_TYPE (*new_var));
-      new_expr = build_conditional_expr
-	(location,
-	 build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
-	 new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+      new_comp_expr = build2 (LT_EXPR, TREE_TYPE (*new_var), *new_var,
+			      build_c_cast (location, TREE_TYPE (*new_var),
+					    func_parm));
+      new_expr = build_conditional_expr (location, new_comp_expr, false,
+					 new_yes_expr, TREE_TYPE (*new_var),
+					 new_no_expr, TREE_TYPE (*new_var));
       break;
     case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN:
       if (TYPE_MAX_VALUE (new_var_type))
@@ -503,10 +505,12 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
       new_yes_expr = build_modify_expr
 	(location, *new_var, TREE_TYPE (*new_var), NOP_EXPR,
 	 location, func_parm, TREE_TYPE (*new_var));
-      new_expr = build_conditional_expr
-	(location,
-	 build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var, func_parm), false,
-	 new_yes_expr, TREE_TYPE (*new_var), new_no_expr, TREE_TYPE (*new_var));
+      new_comp_expr = build2 (GT_EXPR, TREE_TYPE (*new_var), *new_var,
+			      build_c_cast (location, TREE_TYPE (*new_var),
+					    func_parm));
+      new_expr = build_conditional_expr (location, new_comp_expr, false,
+					 new_yes_expr, TREE_TYPE (*new_var),
+					 new_no_expr, TREE_TYPE (*new_var));
       break;
     case BUILT_IN_CILKPLUS_SEC_REDUCE_MAX_IND:
       new_var_init = build_modify_expr
@@ -551,12 +555,13 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
       append_to_statement_list (new_no_ind, &new_no_list);
       append_to_statement_list (new_no_expr, &new_no_list);
  
+      new_comp_expr =
+	build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
+		build_c_cast (location, TREE_TYPE (array_ind_value),
+			      func_parm));
       new_expr = build_conditional_expr
-	(location,
-	 build2 (LE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
-		 func_parm),
-	 false,
-	 new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+	(location, new_comp_expr, false,  new_yes_list, TREE_TYPE (*new_var),
+	 new_no_list, TREE_TYPE (*new_var));
       break;
     case BUILT_IN_CILKPLUS_SEC_REDUCE_MIN_IND:
       new_var_init = build_modify_expr
@@ -601,24 +606,34 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
       append_to_statement_list (new_no_ind, &new_no_list);
       append_to_statement_list (new_no_expr, &new_no_list);
  
+      new_comp_expr =
+	build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
+		build_c_cast (location, TREE_TYPE (array_ind_value),
+			      func_parm));
       new_expr = build_conditional_expr
-	(location,
-	 build2 (GE_EXPR, TREE_TYPE (array_ind_value), array_ind_value,
-		 func_parm),
-	 false,
-	 new_yes_list, TREE_TYPE (*new_var), new_no_list, TREE_TYPE (*new_var));
+	(location, new_comp_expr, false, new_yes_list, TREE_TYPE (*new_var),
+	 new_no_list, TREE_TYPE (*new_var));
       break;
     case BUILT_IN_CILKPLUS_SEC_REDUCE:
       new_var_init = build_modify_expr
 	(location, *new_var, TREE_TYPE (*new_var), NOP_EXPR,
 	 location, identity_value, new_var_type);
-      new_call_expr = build_call_expr (call_fn, 2, *new_var, func_parm);
+      new_call_expr = build_call_expr (call_fn, 2, *new_var,
+				       build_c_cast (location, new_var_type,
+						     func_parm));
       new_expr = build_modify_expr
 	(location, *new_var, TREE_TYPE (*new_var), NOP_EXPR,
 	 location, new_call_expr, TREE_TYPE (*new_var));
       break;
     case BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING:
-      new_expr = build_call_expr (call_fn, 2, identity_value, func_parm);
+      if (TREE_CODE (TREE_TYPE (identity_value)) == POINTER_TYPE)
+	new_var_type = TREE_TYPE (TREE_TYPE (identity_value));
+      identity_expr = TREE_OPERAND (identity_value, 0);
+      identity_value = build_fold_addr_expr (identity_expr);
+      TREE_USED (identity_expr) = 1;
+      new_expr = build_call_expr (call_fn, 2, identity_value,
+				  build_c_cast (location, new_var_type,
+						func_parm));
       break;
     default:
       gcc_unreachable ();
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b8fe362..6c6c50c 100644
Binary files a/gcc/testsuite/ChangeLog and b/gcc/testsuite/ChangeLog differ
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/builtin_fn_mutating.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/builtin_fn_mutating.c
index 6635565..7c194c2 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/builtin_fn_mutating.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/builtin_fn_mutating.c
@@ -44,11 +44,11 @@ int main(void)
   max_value = array3[0] * array4[0];
   for (ii = 0; ii < 10; ii++)
     if (array3[ii] * array4[ii] > max_value) {
-      max_value = array3[ii] * array4[ii];
       max_index = ii;
     }
     
-  
+  for (ii = 0; ii < 10; ii++)
+    my_func (&max_value, array3[ii] * array4[ii]);
   
 #if HAVE_IO
   for (ii = 0; ii < 10; ii++) 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-06-10 22:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09  1:20 [PATCH] Fix for PR c/57563 Iyer, Balaji V
2013-06-10 14:39 ` Joseph S. Myers
2013-06-10 15:08   ` Iyer, Balaji V
2013-06-10 15:16     ` Joseph S. Myers
2013-06-10 17:19       ` Iyer, Balaji V
2013-06-10 21:18         ` Joseph S. Myers
2013-06-10 22:16           ` Iyer, Balaji V
2013-06-10 22:45             ` Joseph S. Myers
2013-06-10 22:18           ` FW: " Iyer, Balaji V

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).