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: ibuc...@gdcproject.org
Reporter: li...@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.
