Thread overview
[Bug 198] Optimization makes incorrect results
Sep 05, 2015
Alex
Sep 14, 2015
Iain Buclaw
Sep 27, 2015
Iain Buclaw
Sep 27, 2015
Iain Buclaw
Oct 03, 2015
Iain Buclaw
Jun 10, 2017
Iain Buclaw
September 05, 2015
http://bugzilla.gdcproject.org/show_bug.cgi?id=198

--- Comment #1 from Alex <alex@sunopti.com> ---
Reproduced it in git gdc-4.9 branch at commit : d0dd4a83de81511389c686d343a4f4c2a50dbcd0

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


September 14, 2015
http://bugzilla.gdcproject.org/show_bug.cgi?id=198

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> ---
Seems to be something bizarre going on with struct member functions, possibly only related to instantiated members.  Having a bare function works just fine.

ref Vector opOpAssign (ref Vector rthis, Vector operand)
{
    rthis.x += operand.x;
    rthis.y += operand.y;
    rthis.z += operand.z;
    return rthis;
}

Either a mismatch between function type and function parameter types are throwing off the optimizer, or something else is afoot to make it believe that the 'this' value members can never change.

In gdc-5, all optimisation levels constfold the answer correctly in the
testcase, except for -Og level, which calls f(), but the entire foreach loop
has been reduced to nothing, so it passes Vector(0,0,0) to sum().

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


September 27, 2015
http://bugzilla.gdcproject.org/show_bug.cgi?id=198

--- Comment #3 from Iain Buclaw <ibuclaw@gdcproject.org> ---
(In reply to Iain Buclaw from comment #2)
> Seems to be something bizarre going on with struct member functions, possibly only related to instantiated members.  Having a bare function works just fine.
> 
> ref Vector opOpAssign (ref Vector rthis, Vector operand)
> {
>     rthis.x += operand.x;
>     rthis.y += operand.y;
>     rthis.z += operand.z;
>     return rthis;
> }
> 
> Either a mismatch between function type and function parameter types are throwing off the optimizer, or something else is afoot to make it believe that the 'this' value members can never change.
> 
> In gdc-5, all optimisation levels constfold the answer correctly in the
> testcase, except for -Og level, which calls f(), but the entire foreach loop
> has been reduced to nothing, so it passes Vector(0,0,0) to sum().

All wrong, I was able to reproduce the bug by applying @forceinline on the function, some deeper digging and I've found via: -Og -finline-functions -fdump-tree-cddce-stats-details:

Eliminating unnecessary statements:
Deleting : sum.z = _50;
Deleting : _50 = _48 + 3.0e+0;
Deleting : _48 = sum.z;
Deleting : sum.y = _47;
Deleting : _47 = _45 + 2.0e+0;
Deleting : _45 = sum.y;
Deleting : sum.x = _44;
Deleting : _44 = _9 + 1.0e+0;
Deleting : _9 = sum.x;
Deleting : operand = D.3725;
Deleting : D.3725 = __ctmp4;
Deleting : __ctmp4.z = 3.0e+0;
Deleting : __ctmp4.y = 2.0e+0;
Deleting : __ctmp4.x = 1.0e+0;
Deleting : __ctmp4 = {};
Deleting : _14 = v_1 <= 2;
Deleting : sum.z = 0.0;
Deleting : sum.y = 0.0;
Deleting : sum.x = 0.0;

And yet it doesn't remove the sum.v[] deferences as it's able to properly associate it with a shared memory location.

Making the anonymous union a named union fixes this indefinitely.  So it seems like it all comes down to lack of association in our anonymous members.  So we should at least give anonymous structs/unions a dummy name, even though people will see it as a flat structure in debug.

So this would be a duplicate of another bug related to unions/structs...

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


September 27, 2015
http://bugzilla.gdcproject.org/show_bug.cgi?id=198

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #4 from Iain Buclaw <ibuclaw@gdcproject.org> ---
Upstream PR raised:

https://github.com/D-Programming-Language/dmd/pull/5122

Doesn't fix the problem, but at least ensures that the code generator has all information it needs to do the right thing.

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


October 03, 2015
http://bugzilla.gdcproject.org/show_bug.cgi?id=198

--- Comment #5 from Iain Buclaw <ibuclaw@gdcproject.org> ---
https://github.com/D-Programming-GDC/GDC/pull/142

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


June 10, 2017
https://bugzilla.gdcproject.org/show_bug.cgi?id=198

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Iain Buclaw <ibuclaw@gdcproject.org> ---
I forgot to close this.

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