> -----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.
Thanks,
Balaji V. Iyer.
>
> jeff
Index: gcc/c/ChangeLog
===================================================================
--- gcc/c/ChangeLog (revision 199672)
+++ gcc/c/ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2013-06-04 Balaji V. Iyer <balaji.v.i...@intel.com>
+
+ * c-typeck.c (convert_arguments): Moved checking of builtin cilkplus
+ reduction functions outside the for-loop. Also, added a check if the
+ fundecl is non-NULL.
+
2013-06-03 Balaji V. Iyer <balaji.v.i...@intel.com>
* c-typeck.c (c_finish_if_stmt): Added a check to see if the rank of the
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;
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog (revision 199672)
+++ gcc/testsuite/ChangeLog (working copy)
@@ -4,6 +4,11 @@
2013-06-04 Balaji V. Iyer <balaji.v.i...@intel.com>
+ PR C/57457
+ * c-c++-common/cilk-plus/AN/pr57457.c: New test.
+
+2013-06-04 Balaji V. Iyer <balaji.v.i...@intel.com>
+
* c-c++-common/cilk-plus/AN/array_test1.c (main): Replaced argc, argv
parameters with void.
(main2): Removed argc parameter.
Index: gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457-2.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457-2.c (revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457-2.c (revision 0)
@@ -0,0 +1,15 @@
+/* { 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" } */
+}
Index: gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c (revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/AN/pr57457.c (revision 0)
@@ -0,0 +1,39 @@
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+/* This test has no array notation components in it and thus should compile
+ fine without crashing. */
+
+typedef unsigned int size_t;
+typedef int (*__compar_fn_t) (const void *, const void *);
+extern void *bsearch (const void *__key, const void *__base,
+ size_t __nmemb, size_t __size, __compar_fn_t
+ __compar)
+ __attribute__ ((__nonnull__ (1, 2, 5))) ;
+extern __inline __attribute__ ((__gnu_inline__)) void *
+bsearch (const void *__key, const void *__base, size_t __nmemb, size_t
+ __size,
+ __compar_fn_t __compar)
+{
+ size_t __l, __u, __idx;
+ const void *__p;
+ int __comparison;
+ __l = 0;
+ __u = __nmemb;
+ while (__l < __u)
+ {
+ __idx = (__l + __u) / 2;
+ __p = (void *) (((const char *) __base) +
+ (__idx * __size));
+ __comparison = (*__compar) (__key,
+ __p);
+ if (__comparison < 0)
+ __u = __idx;
+ else if (__comparison > 0)
+ __l = __idx + 1;
+ else
+ return (void *)
+ __p;
+ }
+ return ((void *)0);
+}
