2012/1/10 Richard Guenther <richard.guent...@gmail.com>:
> On Tue, Jan 10, 2012 at 10:58 AM, Kai Tietz <ktiet...@googlemail.com> wrote:
>> Ping
>>
>> 2012/1/8 Kai Tietz <ktiet...@googlemail.com>:
>>> Hi,
>>>
>>> this patch makes sure that for increment of
>>> postfix-increment/decrement we use also orignal lvalue instead of tmp
>>> lhs value for increment.  This fixes reported issue about sequence
>>> point in PR/48814
>>>
>>> ChangeLog
>>>
>>> 2012-01-08  Kai Tietz  <kti...@redhat.com>
>>>
>>>          PR middle-end/48814
>>>          * gimplify.c (gimplify_self_mod_expr): Use for
>>> postfix-inc/dec lvalue instead of temporary
>>>          lhs.
>>>
>>> Regression tested for x86_64-unknown-linux-gnu for all languages
>>> (including Ada and Obj-C++).  Ok for apply?
>>>
>>> Regards,
>>> Kai
>>>
>>> Index: gimplify.c
>>> ===================================================================
>>> --- gimplify.c  (revision 182720)
>>> +++ gimplify.c  (working copy)
>>> @@ -2258,7 +2258,7 @@
>>>       arith_code = POINTER_PLUS_EXPR;
>>>     }
>>>
>>> -  t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs);
>>> +  t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs);
>>>
>>>   if (postfix)
>>>     {

Hi Richard,

> Please add testcases.  Why does your patch make a difference?
> lhs is just the gimplified lvalue.

yes, exactly this makes a big difference for post-inc/dec.  I show you
gimple-dump to illustrate this in more detail.  I used here -O2 option
with using attached patch.

gcc without that patch produces following gimple for main:

main ()
{
  int count.0;
  int count.1;
  int D.2721;
  int D.2725;
  int D.2726;

  count.0 = count; <-- here we store orginal value 'count' for having
array-access-index
  D.2721 = incr (); <- within that function count gets modified
  arr[count.0] = D.2721;
  count.1 = count.0 + 1; <- Here happens the issue.  We increment the
saved value of 'count'
  count = count.1; <- By this the modification of count in incr() gets void.
  ...

By the change we make sure to use count's value instead its saved temporary.

Patched gcc produces this gimple:

main ()
{
  int count.0;
  int count.1;
  int D.1718;
  int D.1722;
  int D.1723;

  count.0 = count;
  D.1718 = incr ();
  arr[count.0] = D.1718;
  count.0 = count; <-- Reload count.0 for post-inc/dec to use count's
current value
  count.1 = count.0 + 1;
  count = count.1;
  count.0 = count;

Ok, here is the patch with adusted testcase from PR.

ChangeLog

2012-01-10  Kai Tietz  <kti...@redhat.com>

        PR middle-end/48814
        * gimplify.c (gimplify_self_mod_expr): Use for
        postfix-inc/dec lvalue instead of temporary lhs.

Regression tested for all languages (including Ada and Obj-C++).  Ok for apply?

Regards,
Kai

2012-01-10  Kai Tietz  <kti...@redhat.com>

        * gcc.c-torture/execute/pr48814.c: New test.

Index: gcc/gcc/gimplify.c
===================================================================
--- gcc.orig/gcc/gimplify.c
+++ gcc/gcc/gimplify.c
@@ -2258,7 +2258,7 @@ gimplify_self_mod_expr (tree *expr_p, gi
       arith_code = POINTER_PLUS_EXPR;
     }

-  t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs);
+  t1 = build2 (arith_code, TREE_TYPE (*expr_p), lvalue, rhs);

   if (postfix)
     {
Index: gcc/gcc/testsuite/gcc.c-torture/execute/pr48814.c
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/gcc.c-torture/execute/pr48814.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;
+}

Reply via email to