[Bug 185] New: Wrong codegen is used for <OP>= expressions when there is a function as part of the rvalue.

[via D.gnu]

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 at gdcproject.org
          Reporter: liran at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/d.gnu/attachments/20150515/f29f0cb9/attachment.html>