Patchwork [cilkplus-merge] test for side effects

login
register
mail settings
Submitter Aldy Hernandez
Date March 20, 2013, 9:59 p.m.
Message ID <514A3153.6040707@redhat.com>
Download mbox | patch
Permalink /patch/229504/
State New
Headers show

Comments

Aldy Hernandez - March 20, 2013, 9:59 p.m.
> I have found some little nits that I will point out in a reply to this
> message.

Balaji:

In Joseph's review on October 19, 2012 
(http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01838.html) he mentioned:

> Say expr1 through expr9 are expressions with side effects, and you have:
>
> expr1[expr2:expr3:expr4] = expr5[expr6:expr7:expr8] + expr9;
>
> The spec says "However, in such a statement, a sub-expression with rank
> zero is evaluated only once." - that is, each of the nine expressions is
> evaluated once.  I don't see any calls to save_expr to ensure these
> semantics, or any testcases that verify that they are adhered to.

If I understand Joseph's comment, this is still broken on the branch. 
For example, in the example below:

   array[func1() + 11 : func2() + 22 : func3() + 33] = 666;

...the function calls should only be called once, yet currently we generate:

   D.1739 = 0;
   <D.1740>:
   D.1769 = func2 ();
   D.1770 = D.1769 + 22;
   if (D.1739 < D.1770) goto <D.1741>; else goto <D.1742>;
   <D.1741>:
   D.1771 = func1 ();		<-- BOO HISS
   D.1772 = D.1771 + 11;
   D.1773 = func3 ();		<-- BOO HISS
   D.1774 = D.1773 + 33;
   D.1775 = D.1739 * D.1774;
   D.1776 = D.1772 + D.1775;
   array[D.1776] = 666;
   D.1739 = D.1739 + 1;
   goto <D.1740>;
   <D.1742>:

As you can see, func1() and func3() are being called repeatedly within 
the loop.

I am adding the attached execute test to the harness.  If my 
understanding is correct, this test should pass.

Joseph, please let me know if I misunderstood things in some way.

Committed to cilkplus-merge branch.
commit 4447dcf380a08f74bf5b91fd84d7013cbbb34ee8
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Wed Mar 20 16:47:02 2013 -0500

    Add new test to verify that the array index, limit, and stride are
    only evaluated once.

Patch

diff --git a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c
new file mode 100644
index 0000000..1845862
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c
@@ -0,0 +1,23 @@ 
+/* Test that the array index, limit, and stride are evaluated only
+   once.  */
+
+int array[1000];
+
+int func1_times = 0;
+int func2_times = 0;
+int func3_times = 0;
+int func1() { func1_times++; return 0; }
+int func2() { func2_times++; return 0; }
+int func3() { func3_times++; return 0; }
+
+int main()
+{
+  array[func1() + 11 : func2() + 22 : func3() + 33] = 666;
+
+  if (func1_times != 1
+      || func2_times != 1
+      || func3_times != 1)
+    abort();
+
+  return 0;
+}