public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, PR81844] Fix condition folding in c_parser_omp_for_loop
@ 2017-08-14  8:43 Tom de Vries
  2017-09-14 10:55 ` Jakub Jelinek
  0 siblings, 1 reply; 8+ messages in thread
From: Tom de Vries @ 2017-08-14  8:43 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches

[-- 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;
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-15 19:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-14  8:43 [PATCH, PR81844] Fix condition folding in c_parser_omp_for_loop Tom de Vries
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

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