• via D.gnu (97/97) May 15 2015 Date: Fri, 15 May 2015 11:03:35 +0100

" via D.gnu" <d.gnu puremagic.com> writes:

Date: Fri, 15 May 2015 11:03:35 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"

http://bugzilla.gdcproject.org/show_bug.cgi?id=185

            Bug ID: 185
           Summary: Wrong codegen is used for <OP>= expressions when there
                    is a function as part of the rvalue.
           Product: GDC
           Version: 4.9.x
          Hardware: x86_64
                OS: Linux
            Status: NEW
          Severity: blocker
          Priority: Normal
         Component: gdc
          Assignee: ibuclaw gdcproject.org
          Reporter: liran weka.io

Initially issue was found in a large Fiber based application, and narrowed down
to use the += operation around functions that later call yield.
The code was:

-----------------
import std.stdio;
import core.thread;

enum numAddition = 3;
long globalSum = 0;

class DerivedFiber : Fiber
{
   int index;
   this(int _index)
   {
       index = _index;
       super( &run );
   }

private :
   void run()
   {
       foreach(int k; 0 .. numAddition) {
           globalSum += otherFunc();
           writefln("DerivedFiber(%d) iteration %d globalSum is %d", index, k,
globalSum);
       }
   }

   long otherFunc() {
       yield();
       return index;
   }
}

int main()
{
   Fiber derived1 = new DerivedFiber(1);
   Fiber derived2 = new DerivedFiber(2);

   foreach(j; 0 .. (numAddition+1)) {
       derived1.call();
       derived2.call();
   }

   assert(globalSum == (1+2)*numAddition);
   return 0;
}
-------------------------
And the problem was that the compiler cached the globalSum value before the
yield, so concurrent changes were not possible.

Later, Johannes Pfau showed that generally <OP>= caches the "origin" even if
calling functions from other modules that may change the value without use of
fibers and yield at all.

"""
Iain: I think our codegen might be wrong. -fdump-tree-original:

 61     var_decl         name:  76      mngl:  77      type:  60     
                        scpe:  54      srcp: test.d:5      
                        size:  39      algn: 64       used: 1 
 62     plus_expr        type:  60      op 0:  61      op 1:  78
 78     call_expr        type:  60      fn  :  96      0   :  23

Does GCC guarantee an evaluation order for plus_expr? This is
kinda weird btw. The rewrite might already happen in the frontend. If we
rewrite
globalSum += otherFunc();
to
globalSum = globalSum + otherFunc();
and follow strict LTR evaluation the code we generate is correct and
the code DMD generates is incorrect.

OTOH I don't know the exact rules for += but intuitively it should
first evaluate the RHS, then load the LHS.

"""


Then Iain Buclaw added:
"""
I can confirm that C codegen does infact emit 'foo += bar()'  as  'foo
= bar() + foo'

Which only strengthens the reasoning to change it.
.
.
.
For commutative operations we can simply change the operands. For
non-commutative operations we'll have to explicitly evaluate the side
effects of the RHS before assigning. (-=, ...)
"""

-- 
You are receiving this mail because:
You are watching all bug changes.