From: Tom de Vries <Tom_deVries@mentor.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH, PR81844] Fix condition folding in c_parser_omp_for_loop
Date: Mon, 14 Aug 2017 08:43:00 -0000 [thread overview]
Message-ID: <8d91b179-6873-2988-529b-705e123010cf@mentor.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]
Hi,
this patch fixes the wrong-code PR81844, where an omp for loop is
incorrectly removed by the compiler.
Consider the test-case from the patch.
It contains a omp for condition 'i > 0x7fffffffffffffffULL', where i is
of type unsigned long long int.
In c_parser_omp_for_loop, we first have:
...
(gdb) call debug_generic_expr (cond)
<<< Unknown tree: c_maybe_const_expr
i >>> > 9223372036854775807
...
Then we execute:
...
15030 cond = c_fully_fold (cond, false, NULL);
...
and we have:
...
(gdb) call debug_generic_expr (cond)
(signed long) i < 0
...
Note that the type of the comparison changed from unsigned to signed.
Subsequently, in c_finish_omp_for we do:
...
/* 2.5.1. The comparison in the condition is computed in
the type of DECL, otherwise the behavior is undefined.
For example:
long n; int i;
i < n;
according to ISO will be evaluated as:
(long)i < n;
We want to force:
i < (int)n; */
if (TREE_CODE (op0) == NOP_EXPR
&& decl == TREE_OPERAND (op0, 0))
{
TREE_OPERAND (cond, 0) = TREE_OPERAND (op0, 0);
TREE_OPERAND (cond, 1)
= fold_build1_loc (elocus, NOP_EXPR,
TREE_TYPE (decl),
TREE_OPERAND (cond, 1));
}
...
and we end up with:
...
(gdb) call debug_generic_expr (cond)
i < 0
...
which is always false.
AFAIU, the problem is that the optimization in c_finish_omp_for assumes
that the operand type of the condition is the same as in the source
code, while c_fully_fold breaks that assumption.
The patch fixes this by folding only the operands of the condition.
Bootstrapped and reg-tested on x86_64.
OK for trunk?
Thanks,
- Tom
[-- Attachment #2: 0001-Fix-condition-folding-in-c_parser_omp_for_loop.patch --]
[-- Type: text/x-patch, Size: 2096 bytes --]
Fix condition folding in c_parser_omp_for_loop
2017-08-14 Tom de Vries <tom@codesourcery.com>
PR c/81844
* c-parser.c (c_parser_omp_for_loop): Fix condition folding.
* testsuite/libgomp.c/pr81805.c: New test.
---
gcc/c/c-parser.c | 19 +++++++++++++++-
libgomp/testsuite/libgomp.c/pr81805.c | 42 +++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index d018fbc..cba4103 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -15027,7 +15027,24 @@ c_parser_omp_for_loop (location_t loc, c_parser *parser, enum tree_code code,
cond = cond_expr.value;
cond = c_objc_common_truthvalue_conversion (cond_loc, cond);
- cond = c_fully_fold (cond, false, NULL);
+ switch (TREE_CODE (cond))
+ {
+ case GT_EXPR:
+ case GE_EXPR:
+ case LT_EXPR:
+ case LE_EXPR:
+ case NE_EXPR:
+ {
+ tree op0 = TREE_OPERAND (cond, 0), op1 = TREE_OPERAND (cond, 1);
+ op0 = c_fully_fold (op0, false, NULL);
+ op1 = c_fully_fold (op1, false, NULL);
+ TREE_OPERAND (cond, 0) = op0;
+ TREE_OPERAND (cond, 1) = op1;
+ }
+ break;
+ default:
+ break;
+ }
switch (cond_expr.original_code)
{
case GT_EXPR:
diff --git a/libgomp/testsuite/libgomp.c/pr81805.c b/libgomp/testsuite/libgomp.c/pr81805.c
new file mode 100644
index 0000000..fa78b3c
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/pr81805.c
@@ -0,0 +1,42 @@
+/* { dg-do run } */
+
+extern void abort (void);
+
+#define N 32ULL
+int a[N];
+
+const unsigned long long c = 0x7fffffffffffffffULL;
+
+void
+f2_tpf_static32 (void)
+{
+ unsigned long long i;
+ #pragma omp for
+ for (i = c + N; i > c; i -= 1ULL)
+ a[i - 1ULL - c] -= 4;
+}
+
+__attribute__((noinline, noclone)) int
+test_tpf_static32 (void)
+{
+ int i, j, k;
+ for (i = 0; i < N; i++)
+ a[i] = i - 25;
+
+ f2_tpf_static32 ();
+
+ for (i = 0; i < N; i++)
+ if (a[i] != i - 29)
+ return 1;
+
+ return 0;
+}
+
+int
+main ()
+{
+ if (test_tpf_static32 ())
+ abort ();
+
+ return 0;
+}
next reply other threads:[~2017-08-14 8:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-14 8:43 Tom de Vries [this message]
2017-09-14 10:55 ` Jakub Jelinek
2017-09-14 19:34 ` de Vries, Tom
2017-09-14 19:38 ` Jakub Jelinek
2017-12-15 9:07 ` [PATCH, libgomp, testsuite] Move tests to libgomp.c-c++-common Tom de Vries
2018-02-15 19:28 ` Jason Merrill
2017-12-10 14:00 ` [PATCH, PR81844] Fix condition folding in c_parser_omp_for_loop Tom de Vries
2017-12-10 14:57 ` Jakub Jelinek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8d91b179-6873-2988-529b-705e123010cf@mentor.com \
--to=tom_devries@mentor.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).