www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.announce - DMD 1.001 release is broken

reply Walter Bright <newshound digitalmars.com> writes:
I checked into the bugs Oskar and a couple others posted, and while 
they're not hard to fix, they do render the release unusable. I'll try 
to get an update as soon as I can, in the meanwhile, stick with 1.00.
Jan 24 2007
parent reply "Chris Miller" <chris dprogramming.com> writes:
On Wed, 24 Jan 2007 19:18:19 -0500, Walter Bright  
<newshound digitalmars.com> wrote:

 I checked into the bugs Oskar and a couple others posted, and while  
 they're not hard to fix, they do render the release unusable. I'll try  
 to get an update as soon as I can, in the meanwhile, stick with 1.00.

I think there may also be something wrong with NRVO (Named Return Value Optimization). Many of my projects just ended up with broken features until I downgraded to DMD 0.177. I was able to find a workaround for one of the things it broke by using an inout parameter instead of returning a struct, but other things were too broken in that project so I still had to downgrade DMD. I don't have reproducable examples, it took me long enough just to track down and confirm that one issue with the workaround.
Jan 24 2007
next sibling parent reply Walter Bright <newshound digitalmars.com> writes:
Chris Miller wrote:
 I think there may also be something wrong with NRVO (Named Return Value 
 Optimization). Many of my projects just ended up with broken features 
 until I downgraded to DMD 0.177. I was able to find a workaround for one 
 of the things it broke by using an inout parameter instead of returning 
 a struct, but other things were too broken in that project so I still 
 had to downgrade DMD. I don't have reproducable examples, it took me 
 long enough just to track down and confirm that one issue with the 
 workaround.

I need a test case. The ones I have all work.
Jan 24 2007
parent reply Frits van Bommel <fvbommel REMwOVExCAPSs.nl> writes:
Walter Bright wrote:
 Chris Miller wrote:
 I think there may also be something wrong with NRVO (Named Return 
 Value Optimization). Many of my projects just ended up with broken 
 features until I downgraded to DMD 0.177. I was able to find a 
 workaround for one of the things it broke by using an inout parameter 
 instead of returning a struct, but other things were too broken in 
 that project so I still had to downgrade DMD. I don't have 
 reproducable examples, it took me long enough just to track down and 
 confirm that one issue with the workaround.

I need a test case. The ones I have all work.

Here's one that fails (tested on DMD v1.00, v1.002): ----- import std.stdio; struct Data { int x, y; /// To make size > 8 so NRVO is used. /// Program runs correctly with this line commented out: byte filler; } Data frob(inout Data d) { Data ret; ret.y = d.x - d.y; ret.x = d.x + d.y; return ret; } void main() { Data d; d.x = 1; d.y = 2; d = frob(d); writefln(d.x); writefln(d.y); assert(d.x == 3 && d.y == -1); } ----- The problem here is return value/parameter aliasing. It initializes the return value before even looking at the parameter...
Jan 25 2007
parent reply Walter Bright <newshound digitalmars.com> writes:
Frits van Bommel wrote:
 Here's one that fails (tested on DMD v1.00, v1.002):
 
 -----
 import std.stdio;
 
 struct Data
 {
     int x, y;
 
     /// To make size > 8 so NRVO is used.
     /// Program runs correctly with this line commented out:
     byte filler;
 }
 
 Data frob(inout Data d)
 {
     Data ret;
     ret.y = d.x - d.y;
     ret.x = d.x + d.y;
     return ret;
 }
 
 void main() {
     Data d; d.x = 1; d.y = 2;
     d = frob(d);
     writefln(d.x);
     writefln(d.y);
     assert(d.x == 3 && d.y == -1);
 }
 -----
 
 The problem here is return value/parameter aliasing. It initializes the 
 return value before even looking at the parameter...

I believe this fails with C++, too. But I don't think there's any solution to it but completely disable NRVO. There's no reasonable way for the compiler to detect that the d's are aliased. But NRVO is too valuable an optimization. So instead I'll argue that this is akin to: i = i++; i.e. it's an order-of-evaluation issue that should be avoided. NRVO aliasing can be avoided by using a temporary: Data ret2 = ret; return ret2; instead of: return ret; Passing d by value instead of by ref will do the same thing. I assume that the reason inout is used here is to pass by reference to gain efficiency. Disabling NRVO will lose more than every bit of efficiency gained by passing by reference - so you might as well not pass it by reference, but by value. (NRVO eliminates two copies of the struct, not one. Replacing inout with in adds one copy. So NRVO is still one copy ahead <g>.)
Jan 25 2007
next sibling parent reply Lionello Lunesu <lio lunesu.remove.com> writes:
Walter Bright wrote:
 Frits van Bommel wrote:
 Here's one that fails (tested on DMD v1.00, v1.002):

 -----
 import std.stdio;

 struct Data
 {
     int x, y;

     /// To make size > 8 so NRVO is used.
     /// Program runs correctly with this line commented out:
     byte filler;
 }

 Data frob(inout Data d)
 {
     Data ret;
     ret.y = d.x - d.y;
     ret.x = d.x + d.y;
     return ret;
 }

 void main() {
     Data d; d.x = 1; d.y = 2;
     d = frob(d);
     writefln(d.x);
     writefln(d.y);
     assert(d.x == 3 && d.y == -1);
 }
 -----

 The problem here is return value/parameter aliasing. It initializes 
 the return value before even looking at the parameter...

I believe this fails with C++, too. But I don't think there's any solution to it but completely disable NRVO. There's no reasonable way for the compiler to detect that the d's are aliased. But NRVO is too valuable an optimization. So instead I'll argue that this is akin to: i = i++; i.e. it's an order-of-evaluation issue that should be avoided.

This still fails: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=46576 (it's also in bugzilla, issue 829) There's no "inout", just: struct Vector3 { //.... Vector3 opMul(float s) { Vector3 ret; ret.x = x*s; ret.y = y*s; ret.z = z*s; return ret; } } Vector3 a; a.set(1,1,1); a = a*2; // a will be nan,nan,nan Where did those nans come from? Must be the initializers from the return value "ret". L.
Jan 25 2007
parent reply Walter Bright <newshound digitalmars.com> writes:
Lionello Lunesu wrote:
 Walter Bright wrote:
 I believe this fails with C++, too. But I don't think there's any 
 solution to it but completely disable NRVO. There's no reasonable way 
 for the compiler to detect that the d's are aliased. But NRVO is too 
 valuable an optimization. So instead I'll argue that this is akin to:
     i = i++;
 i.e. it's an order-of-evaluation issue that should be avoided.

This still fails: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmar .D&article_id=46576 (it's also in bugzilla, issue 829) There's no "inout", just: struct Vector3 { //.... Vector3 opMul(float s) { Vector3 ret; ret.x = x*s; ret.y = y*s; ret.z = z*s; return ret; } } Vector3 a; a.set(1,1,1); a = a*2; // a will be nan,nan,nan Where did those nans come from? Must be the initializers from the return value "ret".

Declaring ret as: Vector3 ret = void; will cure the problem. But I agree it's a problem, because it looks like it should work.
Jan 26 2007
parent Walter Bright <newshound digitalmars.com> writes:
This stinks, I'll fix it.
Jan 26 2007
prev sibling parent Frits van Bommel <fvbommel REMwOVExCAPSs.nl> writes:
Walter Bright wrote:
 Frits van Bommel wrote:
 Here's one that fails (tested on DMD v1.00, v1.002):

 -----


 -----

 The problem here is return value/parameter aliasing. It initializes 
 the return value before even looking at the parameter...

I believe this fails with C++, too. But I don't think there's any solution to it but completely disable NRVO. There's no reasonable way for the compiler to detect that the d's are aliased. But NRVO is too

Maybe not detect, but it can disable NRVO if there's an inout parameter with the same (or in case of Objects, related) type as the return value. Not sure if that's a good idea though, since it may not be needed in the majority of cases. [later] Maybe it could just automatically create the temp variable if the place to put the return value is also passed by reference to the function? Then you'd still get the benefit of NRVO for calls that don't do that and you get the benefit of working code for calls that do ;). Of course, that only solves _that_ particular aliasing problem...
 valuable an optimization. So instead I'll argue that this is akin to:
     i = i++;
 i.e. it's an order-of-evaluation issue that should be avoided.

I disagree. Function calls are a well-established operation to serialize operations (C++'s sequence points). Conceptually, the assignment happens after the return and optimizations should not affect that. IMHO, at least. (And it would seem "In G++'s Humble Opinion" as well, see below)
 NRVO aliasing can be avoided by using a temporary:
     Data ret2 = ret;
     return ret2;
 instead of:
     return ret;
 Passing d by value instead of by ref will do the same thing. I assume 
 that the reason inout is used here is to pass by reference to gain 
 efficiency. Disabling NRVO will lose more than every bit of efficiency 
 gained by passing by reference - so you might as well not pass it by 
 reference, but by value. (NRVO eliminates two copies of the struct, not 
 one. Replacing inout with in adds one copy. So NRVO is still one copy 
 ahead <g>.)

As I mentioned above, I believe this temporary could be created automatically when necessary. Now, about your statement:
 I believe this fails with C++, too.

Not with g++[1]. I even tried calling the D function from C++ (since the g++-compiled translation happens to read both values before writing to them) and *that* worked fine, so it would seem g++ fixes it in the caller: ----- urxae urxae:~/tmp$ cat callfrob.cpp #include <iostream> #include <cassert> struct Data { int x, y; /// To make size > 8 so NRVO is used. /// Program runs correctly with this line commented out: char filler; }; extern "C" Data frob(Data& d); extern"C" void callfrob() { using namespace std; Data d; d.x = 1; d.y = 2; d = frob(d); cout << "C++:" << endl << d.x << endl << d.y << endl; assert(d.x == 3 && d.y == -1); } urxae urxae:~/tmp$ g++ -c callfrob.cpp -o callfrob.o urxae urxae:~/tmp$ cat frob.d import std.stdio; struct Data { int x, y; /// To make size > 8 so NRVO is used. /// Program runs correctly with this line commented out: byte filler; } extern(C) Data frob(inout Data d) { Data ret; ret.y = d.x - d.y; ret.x = d.x + d.y; return ret; } extern(C) void callfrob(); void main() { Data d; d.x = 1; d.y = 2; d = frob(d); writefln("D:"); writefln(d.x); writefln(d.y); callfrob(); } urxae urxae:~/tmp$ dmd frob.d callfrob.o -L-lstdc++ -offrob gcc frob.o callfrob.o -o frob -m32 -lphobos -lpthread -lm -Xlinker -lstdc++ -Xlinker -L/home/urxae/opt/dmd/lib urxae urxae:~/tmp$ ./frob D: 0 0 C++: 3 -1 ----- The relevant part of callfrob.o's 'objdump -Sr': ----- 7a: c7 45 ec 01 00 00 00 movl $0x1,0xffffffec(%ebp) 81: c7 45 f0 02 00 00 00 movl $0x2,0xfffffff0(%ebp) 88: 8d 55 d8 lea 0xffffffd8(%ebp),%edx 8b: 8d 45 ec lea 0xffffffec(%ebp),%eax 8e: 89 44 24 04 mov %eax,0x4(%esp) 92: 89 14 24 mov %edx,(%esp) 95: e8 fc ff ff ff call 96 <callfrob+0x24> 96: R_386_PC32 frob 9a: 83 ec 04 sub $0x4,%esp 9d: 8b 45 d8 mov 0xffffffd8(%ebp),%eax a0: 89 45 ec mov %eax,0xffffffec(%ebp) a3: 8b 45 dc mov 0xffffffdc(%ebp),%eax a6: 89 45 f0 mov %eax,0xfffffff0(%ebp) a9: 8b 45 e0 mov 0xffffffe0(%ebp),%eax ac: 89 45 f4 mov %eax,0xfffffff4(%ebp) ----- So it would seem g++ in this situation indeed creates a temporary "variable" on the stack, uses it for the return value, and then copies it over the actual variable. Presumably it does this for a reason. Either it's required by the C++ standard or they thought it'd be polite to DWIM in this situation... And no, it doesn't do that if I change the line with the call to 'Data e = frob(d)' (and the output to show e.x & e.y): ----- 7a: c7 45 ec 01 00 00 00 movl $0x1,0xffffffec(%ebp) 81: c7 45 f0 02 00 00 00 movl $0x2,0xfffffff0(%ebp) 88: 8d 55 e0 lea 0xffffffe0(%ebp),%edx 8b: 8d 45 ec lea 0xffffffec(%ebp),%eax 8e: 89 44 24 04 mov %eax,0x4(%esp) 92: 89 14 24 mov %edx,(%esp) 95: e8 fc ff ff ff call 96 <callfrob+0x24> 96: R_386_PC32 frob 9a: 83 ec 04 sub $0x4,%esp 9d: 8b 75 e4 mov 0xffffffe4(%ebp),%esi a0: 8b 5d e0 mov 0xffffffe0(%ebp),%ebx ----- Those last two instructions were included to show the absence of the copy instructions, the first disassembly had similar ones: ----- af: 8b 75 f0 mov 0xfffffff0(%ebp),%esi b2: 8b 5d ec mov 0xffffffec(%ebp),%ebx ----- (Presumably, they're preparations for the output calls) And by the way, I also compiled the C++ with -O2 and with -O3. It still performed the copy, just more efficiently ;): ----- 93: e8 fc ff ff ff call 94 <callfrob+0x24> 94: R_386_PC32 frob 98: 8b 5d d8 mov 0xffffffd8(%ebp),%ebx 9b: 8b 75 dc mov 0xffffffdc(%ebp),%esi 9e: 8b 45 e0 mov 0xffffffe0(%ebp),%eax a1: 89 5d ec mov %ebx,0xffffffec(%ebp) a4: 89 75 f0 mov %esi,0xfffffff0(%ebp) a7: 83 ec 04 sub $0x4,%esp aa: 89 45 f4 mov %eax,0xfffffff4(%ebp) ----- Versions used to test: ----- urxae urxae:~/tmp$ g++ --version | head -n 1 g++ (GCC) 4.1.2 20060928 (prerelease) (Ubuntu 4.1.1-13ubuntu5) urxae urxae:~/tmp$ dmd | head -n 1 Digital Mars D Compiler v1.002 ----- [1]: g++ is the only C++ compiler I have. If it doesn't work in DMC, I humbly suggest you fix that one too :P.
Jan 26 2007
prev sibling parent reply kris <foo bar.com> writes:
Chris Miller wrote:
 On Wed, 24 Jan 2007 19:18:19 -0500, Walter Bright  
 <newshound digitalmars.com> wrote:
 
 I checked into the bugs Oskar and a couple others posted, and while  
 they're not hard to fix, they do render the release unusable. I'll 
 try  to get an update as soon as I can, in the meanwhile, stick with 
 1.00.

I think there may also be something wrong with NRVO (Named Return Value Optimization). Many of my projects just ended up with broken features until I downgraded to DMD 0.177. I was able to find a workaround for one of the things it broke by using an inout parameter instead of returning a struct, but other things were too broken in that project so I still had to downgrade DMD. I don't have reproducable examples, it took me long enough just to track down and confirm that one issue with the workaround.

We've got a similar issue with the stack being trashed after a call to mktime(&tm) ... used to work just fine ... (linux compiler) Please, can we get a fix for this asap? Or tell us a workaround? It's a showstopper for us
Jan 27 2007
next sibling parent kris <foo bar.com> writes:
kris wrote:
 Chris Miller wrote:
 
 On Wed, 24 Jan 2007 19:18:19 -0500, Walter Bright  
 <newshound digitalmars.com> wrote:

 I checked into the bugs Oskar and a couple others posted, and while  
 they're not hard to fix, they do render the release unusable. I'll 
 try  to get an update as soon as I can, in the meanwhile, stick with 
 1.00.

I think there may also be something wrong with NRVO (Named Return Value Optimization). Many of my projects just ended up with broken features until I downgraded to DMD 0.177. I was able to find a workaround for one of the things it broke by using an inout parameter instead of returning a struct, but other things were too broken in that project so I still had to downgrade DMD. I don't have reproducable examples, it took me long enough just to track down and confirm that one issue with the workaround.

We've got a similar issue with the stack being trashed after a call to mktime(&tm) ... used to work just fine ... (linux compiler) Please, can we get a fix for this asap? Or tell us a workaround? It's a showstopper for us

On second thoughts, this may be a different issue ... the codegen looks ok here. Sorry for jumping to conclusions
Jan 27 2007
prev sibling parent Walter Bright <newshound digitalmars.com> writes:
kris wrote:
 We've got a similar issue with the stack being trashed after a call to 
 mktime(&tm)  ...  used to work just fine  ... (linux compiler)
 
 Please, can we get a fix for this asap? Or tell us a workaround? It's a 
 showstopper for us

Try it on 1.004. If that still fails, I need a test case.
Jan 27 2007