This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Ping: Re: [patch middle-end]: Fix PR/48814 - [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result


2012/1/11 Richard Guenther <richard.guenther@gmail.com>:
>
> count despite being declared volatile and only loaded once in the source
> is loaded twice in gimple. ?If it were a HW register which destroys the
> device after the 2nd load without an intervening store you'd wrecked
> the device ;)
>
> Richard.

Thanks for explaination.  I tried to flip order for lhs/rhs in
gimplify_modify_expr & co.  Issue here is that for some cases we are
relying here on lhs for gimplifying rhs (is_gimple_reg_rhs_or_call vs
is_gimple_mem_rhs_or_call) and this doesn't work for cases in C++
like:

typedef const unsigned char _Jv_Utf8Const;
typedef __SIZE_TYPE__ uaddr;

void maybe_adjust_signature (_Jv_Utf8Const *&s, uaddr &special)
{
  union {
    _Jv_Utf8Const *signature;
    uaddr signature_bits;
  };
  signature = s;
  special = signature_bits & 1;
  signature_bits -= special;
  s = signature;
}

So I modified gimplify_self_mod_expr for post-inc/dec so that we use
following sequence
and add it to pre_p for it:

tmp = lhs;
lvalue = tmp (+/-) rhs
*expr_p = tmp;

ChangeLog

2012-02-08  Kai Tietz  <ktietz@redhat.com>

	PR middle-end/48814
	* gimplify.c (gimplify_self_mod_expr): Move for
	postfix-inc/dec the modification in pre and return
	temporary with origin value.

2012-02-08  Kai Tietz  <ktietz@redhat.com>

	* gcc.c-torture/execute/pr48814-1.c: New test.
	* gcc.c-torture/execute/pr48814-2.c: New test.
	* gcc.dg/tree-ssa/assign-1.c: New test.
	* gcc.dg/tree-ssa/assign-2.c: New test.

I did boostrap for all languages (including Ada and Obj-C++) and
regression tests on host x86_64-unknown-linux-gnu.  Ok for apply?

Regards,
Kai

Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c
+++ gcc/gcc/gimplify.c
@@ -2197,7 +2197,7 @@ gimplify_self_mod_expr (tree *expr_p, gi
 			bool want_value)
 {
   enum tree_code code;
-  tree lhs, lvalue, rhs, t1;
+  tree lhs, lvalue, rhs, t1, t2;
   gimple_seq post = NULL, *orig_post_p = post_p;
   bool postfix;
   enum tree_code arith_code;
@@ -2264,20 +2264,23 @@ gimplify_self_mod_expr (tree *expr_p, gi
       arith_code = POINTER_PLUS_EXPR;
     }

-  t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs);
-
-  if (postfix)
-    {
-      gimplify_assign (lvalue, t1, orig_post_p);
-      gimplify_seq_add_seq (orig_post_p, post);
-      *expr_p = lhs;
-      return GS_ALL_DONE;
-    }
-  else
+  if (!postfix)
     {
+      t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs);
       *expr_p = build2 (MODIFY_EXPR, TREE_TYPE (lvalue), lvalue, t1);
       return GS_OK;
     }
+
+  /* Assign lhs to temporary variable.  */
+  t2 = create_tmp_var (TREE_TYPE (lhs), NULL);
+  gimplify_assign (t2, lhs, pre_p);
+  /* Do increment and assign it to lvalue.  */
+  t1 = build2 (arith_code, TREE_TYPE (*expr_p), t2, rhs);
+  gimplify_assign (lvalue, t1, pre_p);
+
+  gimplify_seq_add_seq (orig_post_p, post);
+  *expr_p = t2;
+  return GS_ALL_DONE;
 }

 /* If *EXPR_P has a variable sized type, wrap it in a WITH_SIZE_EXPR.  */
Index: gcc/gcc/testsuite/gcc.c-torture/execute/pr48814-1.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.c-torture/execute/pr48814-1.c
@@ -0,0 +1,18 @@
+extern void abort (void);
+
+int arr[] = {1,2,3,4};
+int count = 0;
+
+int __attribute__((noinline))
+incr (void)
+{
+  return ++count;
+}
+
+int main()
+{
+  arr[count++] = incr ();
+  if (count != 2 || arr[count] != 3)
+    abort ();
+  return 0;
+}
Index: gcc/gcc/testsuite/gcc.c-torture/execute/pr48814-2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.c-torture/execute/pr48814-2.c
@@ -0,0 +1,18 @@
+extern void abort (void);
+
+int arr[] = {1,2,3,4};
+int count = 0;
+
+int
+incr (void)
+{
+  return ++count;
+}
+
+int main()
+{
+  arr[count++] = incr ();
+  if (count != 2 || arr[count] != 3)
+    abort ();
+  return 0;
+}
Index: gcc/gcc/testsuite/gcc.dg/tree-ssa/assign-1.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/tree-ssa/assign-1.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+volatile int count;
+void bar(int);
+void foo()
+{
+ bar(count++);
+}
+
+/* { dg-final { scan-tree-dump-times "count =" 1 "optimized"} } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: gcc/gcc/testsuite/gcc.dg/tree-ssa/assign-2.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.dg/tree-ssa/assign-2.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+volatile int count;
+int arr[4];
+void foo()
+{
+ arr[count++] = 0;
+}
+
+/* { dg-final { scan-tree-dump-times "count =" 1 "optimized"} } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]