On 06/05/13 09:18, Iyer, Balaji V wrote:
-----Original Message-----
From: Jeff Law [mailto:l...@redhat.com]
Sent: Tuesday, June 04, 2013 11:49 PM
To: Iyer, Balaji V
Cc: gcc-patches@gcc.gnu.org; Steve Ellcey
Subject: Re: [PATCH] pr57457
On 06/04/13 12:58, Iyer, Balaji V wrote:
Actually, you can eliminate the entire if-statement (i.e. remove
if-statement and make the body unconditional). This is because, if
flag_enable_cilkplus is true and is_cilkplus_reduce_builtin (fundecl)
is true, then it would have returned vec_safe_length(values) and will
not even get to this point in the first place. So, this is technically
equivalent to if (1). So, can I remove that and check it in also? It
passes all my regression tests.
I originally thought it could be eliminated as well, but after further
reflection I
couldn't convince myself it'd do the right thing for the case when
flag_enable_cilkplus is true but is_cilkplus_reduce_builtin was false.
Note that triggering that code my be nontrivial, AFAICT we're suppressing a
diagnostic. So you're going to need to write invalid code to get into that
condition at the bottom of the loop at all.
Hi Jeff,
This email is going to be a bit long, so I apologize for it ahead of
time.
Here is the diff of c-typeck.c (to show that I have removed the
unwanted if statement)
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c (revision 199672)
+++ gcc/c/c-typeck.c (working copy)
@@ -2942,6 +2942,8 @@
break;
}
}
+ if (flag_enable_cilkplus && fundecl && is_cilkplus_reduce_builtin (fundecl))
+ return vec_safe_length (values);
/* Scan the given expressions and types, producing individual
converted arguments. */
@@ -2959,17 +2961,6 @@
bool npc;
tree parmval;
- // FIXME: I assume this code is here to handle the overloaded
- // behavior of the __sec_reduce* builtins, and avoid giving
- // argument mismatch warnings/errors. We should probably handle
- // this with the resolve_overloaded_builtin infrastructure.
- /* If the function call is a builtin function call, then we do not
- worry about it since we break them up into its equivalent later and
- we do the appropriate checks there. */
- if (flag_enable_cilkplus
- && is_cilkplus_reduce_builtin (fundecl))
- continue;
-
if (type == void_type_node)
{
if (selector)
@@ -3207,16 +3198,10 @@
if (typetail != 0 && TREE_VALUE (typetail) != void_type_node)
{
- /* If array notation is used and Cilk Plus is enabled, then we do not
- worry about this error now. We will handle them in a later place. */
- if (!flag_enable_cilkplus
- || !is_cilkplus_reduce_builtin (fundecl))
- {
- error_at (input_location,
- "too few arguments to function %qE", function);
- inform_declaration (fundecl);
- return -1;
- }
+ error_at (input_location,
+ "too few arguments to function %qE", function);
+ inform_declaration (fundecl);
+ return -1;
}
return error_args ? -1 : (int) parmnum;
=============================================================================
Here is the testcode that I have written that should trigger the "too few arguments
to function error" with the appropriate note to say where the function declaration
is.
----------------------------------
/* { dg-do compile } */
/* { dg-options "-fcilkplus" } */
/* Test-case contains no array notation but is compiled with -fcilkplus.
It will still print the too few arguments func, thereby saying the
if-statement after the for-loop to check for !flag_enable_cilkplus ||
!is_cilkplus_reduce_function (fundecl) is not valid is always taken. */
int func (int, int); /* { dg-message "declared here" } */
int main (void)
{
int a = 5, b = 2;
return func (a); /* { dg-error "too few arguments to function" } */
}
--------------------------------
Here is the output when I run this:
bviyer@lakshmi2a:/export/users/gcc-svn/b-trunk-gcc/gcc>
../../install_dir/trunk-install/bin/gcc -fcilkplus test.c
test.c: In function âmainâ:
test.c:14:3: error: too few arguments to function âfuncâ
return func (a); /* { dg-error "too few arguments to function" } */
^
test.c:9:5: note: declared here
int func (int, int); /* { dg-message "declared here" } */
^
The whole patch with testcode is attached, so is this Ok for trunk?
Here are the ChangeLog entries:
gcc/c/ChangeLog
2013-06-05 Balaji V. Iyer <balaji.v.i...@intel.com>
* c-typeck.c (convert_arguments): Moved checking of builtin cilkplus
reduction functions outside the for-loop. Added a check if the fundecl
is non-NULL. Finally, removed an unwanted if-statement, and made the
body unconditional
gcc/testsuite/ChangeLog
2013-06-05 Balaji V. Iyer <balaji.v.i...@intel.com>
PR C/57457
* c-c++-common/cilk-plus/AN/pr57457.c: New test.
* c-c++-common/cilk-plus/AN/pr57457-2.c: Likewise.
This is fine. Please install. Thanks!
jeff
