On Thu, 14 Jan 2016, Jeff Law wrote:
On 01/10/2016 08:55 PM, Patrick Palka wrote:
On Mon, Jan 4, 2016 at 1:35 PM, Jeff Law <l...@redhat.com> wrote:
On 01/02/2016 04:26 PM, Patrick Palka wrote:
On Sat, Jan 2, 2016 at 3:21 AM, Jakub Jelinek <ja...@redhat.com> wrote:
On Fri, Jan 01, 2016 at 10:06:34PM -0700, Jeff Law wrote:
gcc/cp/ChangeLog:
* cp-array-notation.c (cp_expand_cond_array_notations): Return
error_mark_node only if find_rank failed, not if it was
successful.
Can you use -fdump-tree-original in the testcase and verify there's no
<<<
error >>> expressions in the resulting dump file?
With that change, this is OK.
I think the patch is incomplete. Because, find_rank does not always
emit
an error if it returns false, so we again have cases where we can get
error_mark_node in the code without error being emitted.
else if (*rank != current_rank)
{
/* In this case, find rank is being recursed through a set
of
expression of the form A <OPERATION> B, where A and B
both
have
array notations in them and the rank of A is not equal to
rank of B.
A simple example of such case is the following: X[:] +
Y[:][:] */
*rank = current_rank;
return false;
}
and other spots. E.g.
if (prev_arg && EXPR_HAS_LOCATION (prev_arg))
error_at (EXPR_LOCATION (prev_arg),
"rank mismatch between %qE and %qE",
prev_arg,
TREE_OPERAND (expr, ii));
looks very suspicious.
Hmm, good point. Here's a contrived test case that causes find_rank to
return false without emitting an error message thus we again end up
with an error_mark_node in the gimplifier:
/* { dg-do compile } */
/* { dg-options "-fcilkplus" } */
void foo() {}
#define ALEN 1024
int main(int argc, char* argv[])
{
typedef void (*f) (void *);
f b[ALEN], c[ALEN][ALEN];
(b[:]) ((void *)c[:][:]);
_Cilk_spawn foo();
return 0;
}
But this patch was intended to only fix the testsuite fallout that
patch 3 would have otherwise caused, and not to e.g. fix all the bugs
with find_rank.
(BTW patch 3 also makes this test case trigger an ICE, instead of
being silently miscompiled.)
Can you please include this test (xfailed) when you commit patch #1. I
think you want the test to scan for error_mark_node in the gimplified
dump.
There are four subdirectories under
gcc/testsuite/c-c++-common/cilk-plus -- AN, CK, PS and SE. Into which
directory should this new xfailed test go?
These are for array notation, so AN seems best.
Jeff
Thanks, this is what I'm going to commit some time tomorrow (assuming no
objections).
I have written the new xfail'd test case to expect that a
"rank mismatch" error ought to have been issued, which I hope is the
correct expectation.
gcc/cp/ChangeLog:
* cp-array-notation.c (cp_expand_cond_array_notations): Return
error_mark_node only if find_rank failed, not if it was
successful.
gcc/testsuite/ChangeLog:
* c-c++-common/cilk-plus/AN/an-if.c: Check that the original
dump does not contain an error_mark_node.
* c-c++-common/cilk-plus/CK/pr60469.c: Likewise.
* c-c++-common/cilk-plus/AN/fn_ptr-2.c: New xfail'd test.
---
gcc/cp/cp-array-notation.c | 4 ++--
gcc/testsuite/c-c++-common/cilk-plus/AN/an-if.c | 5 ++++-
gcc/testsuite/c-c++-common/cilk-plus/AN/fn_ptr-2.c | 14 ++++++++++++++
gcc/testsuite/c-c++-common/cilk-plus/CK/pr60469.c | 5 ++++-
4 files changed, 24 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/cilk-plus/AN/fn_ptr-2.c
diff --git a/gcc/cp/cp-array-notation.c b/gcc/cp/cp-array-notation.c
index f7a4598..4687ced 100644
--- a/gcc/cp/cp-array-notation.c
+++ b/gcc/cp/cp-array-notation.c
@@ -807,8 +807,8 @@ cp_expand_cond_array_notations (tree orig_stmt)
if (!find_rank (EXPR_LOCATION (cond), cond, cond, true, &cond_rank)
|| !find_rank (EXPR_LOCATION (yes_expr), yes_expr, yes_expr, true,
&yes_rank)
- || find_rank (EXPR_LOCATION (no_expr), no_expr, no_expr, true,
- &no_rank))
+ || !find_rank (EXPR_LOCATION (no_expr), no_expr, no_expr, true,
+ &no_rank))
return error_mark_node;
/* If the condition has a zero rank, then handle array notations in body
separately. */
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/an-if.c
b/gcc/testsuite/c-c++-common/cilk-plus/AN/an-if.c
index 4bf85b5..4ac46ab 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/an-if.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/an-if.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-fcilkplus" } */
+/* { dg-options "-fcilkplus -fdump-tree-original" } */
#if HAVE_IO
#include <stdio.h>
@@ -46,3 +46,6 @@ int main() {
}
return 0;
}
+
+/* The C++ FE once emitted a bogus error_mark_node for this test case. */
+/* { dg-final { scan-tree-dump-not "<<< error >>>" "original" } } */
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/fn_ptr-2.c
b/gcc/testsuite/c-c++-common/cilk-plus/AN/fn_ptr-2.c
new file mode 100644
index 0000000..4e1990f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/fn_ptr-2.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+
+typedef void (*f) (void *);
+f b[1024];
+void *c[1024][1024];
+
+int
+main (void)
+{
+ (b[:]) (c[:][:]); /* { dg-error "rank mismatch" "" { xfail *-*-* } } */
+ return 0;
+}
+
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60469.c
b/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60469.c
index ca0cf7f..670df17 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60469.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/CK/pr60469.c
@@ -1,6 +1,6 @@
/* PR middle-end/60469 */
/* { dg-do compile } */
-/* { dg-options "-fcilkplus" } */
+/* { dg-options "-fcilkplus -fdump-tree-original" } */
void foo() {}
@@ -13,3 +13,6 @@ int main(int argc, char* argv[])
_Cilk_spawn foo();
return 0;
}
+
+/* The C++ FE once emitted a bogus error_mark_node for this test case. */
+/* { dg-final { scan-tree-dump-not "<<< error >>>" "original" } } */
--
2.7.0.83.gdfccd77.dirty
