public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
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;
+}

             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).