* [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
* Re: [PATCH] Fix for PR c/57563
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
0 siblings, 1 reply; 9+ messages in thread
From: Joseph S. Myers @ 2013-06-10 14:39 UTC (permalink / raw)
To: Iyer, Balaji V; +Cc: gcc-patches, Jakub Jelinek, mpolacek
On Sun, 9 Jun 2013, Iyer, Balaji V wrote:
> 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.
You don't say what the actual error was, and neither does the original PR.
But if it was an ICE from an EXCESS_PRECISION_EXPR getting to the
gimplifier, that suggests that c_fully_fold isn't getting called somewhere
it should be - and probably calling c_fully_fold is the correct fix rather
than inserting a cast. If you can get such ICEs for
EXCESS_PRECISION_EXPR, it's quite possible you might get them for
C_MAYBE_CONST_EXPR as well (e.g. try using 0 / 0, or compound literals of
variably modified type, in various places in the affected expressions),
which should be fixed by using c_fully_fold but not by inserting a cast.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix for PR c/57563
2013-06-10 14:39 ` Joseph S. Myers
@ 2013-06-10 15:08 ` Iyer, Balaji V
2013-06-10 15:16 ` Joseph S. Myers
0 siblings, 1 reply; 9+ messages in thread
From: Iyer, Balaji V @ 2013-06-10 15:08 UTC (permalink / raw)
To: Joseph Myers; +Cc: gcc-patches, Jakub Jelinek, mpolacek
> -----Original Message-----
> From: Joseph Myers [mailto:joseph@codesourcery.com]
> Sent: Monday, June 10, 2013 10:40 AM
> To: Iyer, Balaji V
> Cc: gcc-patches@gcc.gnu.org; Jakub Jelinek; mpolacek@gcc.gnu.org
> Subject: Re: [PATCH] Fix for PR c/57563
>
> On Sun, 9 Jun 2013, Iyer, Balaji V wrote:
>
> > 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.
>
> You don't say what the actual error was, and neither does the original PR.
> But if it was an ICE from an EXCESS_PRECISION_EXPR getting to the gimplifier,
> that suggests that c_fully_fold isn't getting called somewhere it should be - and
> probably calling c_fully_fold is the correct fix rather than inserting a cast. If you
> can get such ICEs for EXCESS_PRECISION_EXPR, it's quite possible you might get
> them for C_MAYBE_CONST_EXPR as well (e.g. try using 0 / 0, or compound
> literals of variably modified type, in various places in the affected expressions),
> which should be fixed by using c_fully_fold but not by inserting a cast.
It was not. It was actually a type mismatch between double and long double caught in verify_gimple_in_seq function. So, is it OK for trunk?
Thanks,
Balaji V. Iyer.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix for PR c/57563
2013-06-10 15:08 ` Iyer, Balaji V
@ 2013-06-10 15:16 ` Joseph S. Myers
2013-06-10 17:19 ` Iyer, Balaji V
0 siblings, 1 reply; 9+ messages in thread
From: Joseph S. Myers @ 2013-06-10 15:16 UTC (permalink / raw)
To: Iyer, Balaji V; +Cc: gcc-patches, Jakub Jelinek, mpolacek
On Mon, 10 Jun 2013, Iyer, Balaji V wrote:
> > You don't say what the actual error was, and neither does the original PR.
> > But if it was an ICE from an EXCESS_PRECISION_EXPR getting to the gimplifier,
> > that suggests that c_fully_fold isn't getting called somewhere it should be - and
> > probably calling c_fully_fold is the correct fix rather than inserting a cast. If you
> > can get such ICEs for EXCESS_PRECISION_EXPR, it's quite possible you might get
> > them for C_MAYBE_CONST_EXPR as well (e.g. try using 0 / 0, or compound
> > literals of variably modified type, in various places in the affected expressions),
> > which should be fixed by using c_fully_fold but not by inserting a cast.
>
> It was not. It was actually a type mismatch between double and long
> double caught in verify_gimple_in_seq function. So, is it OK for trunk?
A cast still doesn't make sense conceptually. Could you give a more
detailed analysis of what the trees look like at this point where you are
inserting this cast, and how you get to a mismatch?
EXCESS_PRECISION_EXPR can be thought of as a conversion operator. It
should only appear at the top level of an expression. At the point where
excess precision should be removed - the value converted to its semantic
type - either the expression with excess precision should be folded using
c_fully_fold (if this is the expression of an expression statement, or
otherwise will go inside a tree that c_fully_fold does not recurse
inside), or the operand of the EXCESS_PRECISION_EXPR should be converted
to the semantic type with the "convert" function. In neither case is
generating a cast appropriate; that's for when the user actually wrote a
cast in their source code.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix for PR c/57563
2013-06-10 15:16 ` Joseph S. Myers
@ 2013-06-10 17:19 ` Iyer, Balaji V
2013-06-10 21:18 ` Joseph S. Myers
0 siblings, 1 reply; 9+ messages in thread
From: Iyer, Balaji V @ 2013-06-10 17:19 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: gcc-patches, Jakub Jelinek, mpolacek
[-- Attachment #1: Type: text/plain, Size: 3033 bytes --]
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Joseph S. Myers
> Sent: Monday, June 10, 2013 11:16 AM
> To: Iyer, Balaji V
> Cc: gcc-patches@gcc.gnu.org; Jakub Jelinek; mpolacek@gcc.gnu.org
> Subject: RE: [PATCH] Fix for PR c/57563
>
> On Mon, 10 Jun 2013, Iyer, Balaji V wrote:
>
> > > You don't say what the actual error was, and neither does the original PR.
> > > But if it was an ICE from an EXCESS_PRECISION_EXPR getting to the
> > > gimplifier, that suggests that c_fully_fold isn't getting called
> > > somewhere it should be - and probably calling c_fully_fold is the
> > > correct fix rather than inserting a cast. If you can get such ICEs
> > > for EXCESS_PRECISION_EXPR, it's quite possible you might get them
> > > for C_MAYBE_CONST_EXPR as well (e.g. try using 0 / 0, or compound
> > > literals of variably modified type, in various places in the affected
> expressions), which should be fixed by using c_fully_fold but not by inserting a
> cast.
> >
> > It was not. It was actually a type mismatch between double and long
> > double caught in verify_gimple_in_seq function. So, is it OK for trunk?
>
> A cast still doesn't make sense conceptually. Could you give a more detailed
> analysis of what the trees look like at this point where you are inserting this cast,
> and how you get to a mismatch?
>
> EXCESS_PRECISION_EXPR can be thought of as a conversion operator. It should
> only appear at the top level of an expression. At the point where excess
> precision should be removed - the value converted to its semantic type - either
> the expression with excess precision should be folded using c_fully_fold (if this is
> the expression of an expression statement, or otherwise will go inside a tree
> that c_fully_fold does not recurse inside), or the operand of the
> EXCESS_PRECISION_EXPR should be converted to the semantic type with the
> "convert" function. In neither case is generating a cast appropriate; that's for
> when the user actually wrote a cast in their source code.
I looked into it a bit more detail. It was an error on my side. I was removing the excess precision expr layer instead of fully folding it. I did that change (i.e. fully fold the expression) and all the errors seem to go away. Here is the fixed patch that fixes PR c/57563. It passes for 32 bit and 64 bit tests. Here are the changelog entries:
gcc/c/ChangeLog
2013-06-10 Balaji V. Iyer <balaji.v.iyer@intel.com>
* c-array-notation.c (fix_builtin_array_notation_fn): Fully folded
excessive precision expressions in function parameters.
gcc/testsuite/ChangeLog
2013-06-10 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.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
[-- Attachment #2: patch_fix_pr57563.txt --]
[-- Type: text/plain, Size: 1441 bytes --]
diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
index b1040da..9298ae0 100644
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -158,10 +158,13 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
func_parm = CALL_EXPR_ARG (an_builtin_fn, 0);
while (TREE_CODE (func_parm) == CONVERT_EXPR
- || TREE_CODE (func_parm) == EXCESS_PRECISION_EXPR
|| TREE_CODE (func_parm) == NOP_EXPR)
func_parm = TREE_OPERAND (func_parm, 0);
+ /* Fully fold any EXCESSIVE_PRECISION EXPR that can occur in the function
+ parameter. */
+ func_parm = c_fully_fold (func_parm, false, NULL);
+
location = EXPR_LOCATION (an_builtin_fn);
if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank))
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
* RE: [PATCH] Fix for PR c/57563
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:18 ` FW: " Iyer, Balaji V
0 siblings, 2 replies; 9+ messages in thread
From: Joseph S. Myers @ 2013-06-10 21:18 UTC (permalink / raw)
To: Iyer, Balaji V; +Cc: gcc-patches, Jakub Jelinek, mpolacek
On Mon, 10 Jun 2013, Iyer, Balaji V wrote:
> I looked into it a bit more detail. It was an error on my side. I was
> removing the excess precision expr layer instead of fully folding it. I
> did that change (i.e. fully fold the expression) and all the errors seem
> to go away. Here is the fixed patch that fixes PR c/57563. It passes for
> 32 bit and 64 bit tests. Here are the changelog entries:
This version is better, but if removing an EXCESS_PRECISION_EXPR there
caused problems, why is it OK to remove CONVERT_EXPR and NOP_EXPR like you
still do - won't that also cause type mismatches (at least if the
conversions are to types that count as sufficiently different for GIMPLE
purposes - say conversions between 32-bit and 64-bit integers)? Maybe you
actually need to fold without removing any such wrappers first at all?
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix for PR c/57563
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
1 sibling, 1 reply; 9+ messages in thread
From: Iyer, Balaji V @ 2013-06-10 22:16 UTC (permalink / raw)
To: Joseph Myers; +Cc: gcc-patches, Jakub Jelinek, mpolacek
[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]
> -----Original Message-----
> From: Joseph Myers [mailto:joseph@codesourcery.com]
> Sent: Monday, June 10, 2013 5:18 PM
> To: Iyer, Balaji V
> Cc: gcc-patches@gcc.gnu.org; Jakub Jelinek; mpolacek@gcc.gnu.org
> Subject: RE: [PATCH] Fix for PR c/57563
>
> On Mon, 10 Jun 2013, Iyer, Balaji V wrote:
>
> > I looked into it a bit more detail. It was an error on my side. I was
> > removing the excess precision expr layer instead of fully folding it.
> > I did that change (i.e. fully fold the expression) and all the errors
> > seem to go away. Here is the fixed patch that fixes PR c/57563. It
> > passes for
> > 32 bit and 64 bit tests. Here are the changelog entries:
>
> This version is better, but if removing an EXCESS_PRECISION_EXPR there caused
> problems, why is it OK to remove CONVERT_EXPR and NOP_EXPR like you still
> do - won't that also cause type mismatches (at least if the conversions are to
> types that count as sufficiently different for GIMPLE purposes - say conversions
> between 32-bit and 64-bit integers)? Maybe you actually need to fold without
> removing any such wrappers first at all?
I looked into it and they were an artifact of previous implementation. Those while loops were not even being entered. Thus, I took them out. Here is a fixed patch.
Thanks,
Balaji V. Iyer.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
[-- Attachment #2: patch_fix_pr57563.txt --]
[-- Type: text/plain, Size: 2099 bytes --]
diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
old mode 100644
new mode 100755
index b1040da..3285969
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -143,25 +143,18 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
|| an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MUTATING)
{
call_fn = CALL_EXPR_ARG (an_builtin_fn, 2);
- while (TREE_CODE (call_fn) == CONVERT_EXPR
- || TREE_CODE (call_fn) == NOP_EXPR)
+ if (TREE_CODE (call_fn) == ADDR_EXPR)
call_fn = TREE_OPERAND (call_fn, 0);
- call_fn = TREE_OPERAND (call_fn, 0);
-
identity_value = CALL_EXPR_ARG (an_builtin_fn, 0);
- while (TREE_CODE (identity_value) == CONVERT_EXPR
- || TREE_CODE (identity_value) == NOP_EXPR)
- identity_value = TREE_OPERAND (identity_value, 0);
func_parm = CALL_EXPR_ARG (an_builtin_fn, 1);
}
else
func_parm = CALL_EXPR_ARG (an_builtin_fn, 0);
- while (TREE_CODE (func_parm) == CONVERT_EXPR
- || TREE_CODE (func_parm) == EXCESS_PRECISION_EXPR
- || TREE_CODE (func_parm) == NOP_EXPR)
- func_parm = TREE_OPERAND (func_parm, 0);
-
+ /* Fully fold any EXCESSIVE_PRECISION EXPR that can occur in the function
+ parameter. */
+ func_parm = c_fully_fold (func_parm, false, NULL);
+
location = EXPR_LOCATION (an_builtin_fn);
if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank))
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
* RE: [PATCH] Fix for PR c/57563
2013-06-10 22:16 ` Iyer, Balaji V
@ 2013-06-10 22:45 ` Joseph S. Myers
0 siblings, 0 replies; 9+ messages in thread
From: Joseph S. Myers @ 2013-06-10 22:45 UTC (permalink / raw)
To: Iyer, Balaji V; +Cc: gcc-patches, Jakub Jelinek, mpolacek
On Mon, 10 Jun 2013, Iyer, Balaji V wrote:
> > This version is better, but if removing an EXCESS_PRECISION_EXPR there caused
> > problems, why is it OK to remove CONVERT_EXPR and NOP_EXPR like you still
> > do - won't that also cause type mismatches (at least if the conversions are to
> > types that count as sufficiently different for GIMPLE purposes - say conversions
> > between 32-bit and 64-bit integers)? Maybe you actually need to fold without
> > removing any such wrappers first at all?
>
> I looked into it and they were an artifact of previous implementation.
> Those while loops were not even being entered. Thus, I took them out.
> Here is a fixed patch.
Thanks, this patch is OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* FW: [PATCH] Fix for PR c/57563
2013-06-10 21:18 ` Joseph S. Myers
2013-06-10 22:16 ` Iyer, Balaji V
@ 2013-06-10 22:18 ` Iyer, Balaji V
1 sibling, 0 replies; 9+ messages in thread
From: Iyer, Balaji V @ 2013-06-10 22:18 UTC (permalink / raw)
To: gcc-patches
Here is the ChangeLog entries. Sorry I forgot to include in my previous email.
gcc/c/ChangeLog
2013-06-10 Balaji V. Iyer <balaji.v.iyer@intel.com>
* c-array-notation.c (fix_builtin_array_notation_fn): Fully folded
excessive precision expressions in function parameters. Also removed
couple unwanted while statements.
gcc/testsuite/ChangeLog
2013-06-10 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.
^ 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).