4506 – Regression(2.034): -O flag breaks some recursive functions.

D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 4506 - Regression(2.034): -O flag breaks some recursive functions.
Summary: Regression(2.034): -O flag breaks some recursive functions.
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 regression
Assignee: No Owner
URL:
Keywords: patch, wrong-code
Depends on:
Blocks:
Reported: 2010年07月25日 08:29 UTC by Peter Alexander
Modified: 2015年06月09日 05:10 UTC (History)
3 users (show)

See Also:


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description Peter Alexander 2010年07月25日 08:29:05 UTC
I wrote a function to count integer partitions. When compiled with dmd test.d, I get the correct result. Adding the -O flag makes the program return the incorrect result:
import std.stdio;
auto P(int n)
{
	if (n < 0) return 0;
	if (n == 0) return 1;
	
	int c = 1;
	int r = 0;
	for (int k = 1; k <= n; ++k)
	{
		r += c * (P(n-k*(3*k-1)/2) + P(n-k*(3*k+1)/2));
		c *= -1;
	}
	return r;
}
void main()
{
	writeln(P(10));
}
$ dmd test.d
$ ./test
42
$ dmd -O test.d
$ ./test
138
Unfortunately I haven't been able find a more minimal reproduction case. In particular, a basic recursive Fibonacci function works fine in both cases.
Using dmd 2.047 on Mac OS X Snow Leopard.
Comment 1 bearophile_hugs 2010年07月25日 09:31:29 UTC
This is very ugly. A reduced test case:
void main() {
 int c = 1;
 for (int k = 0; k < 2; k++) {
 assert((k == 0 && c == 1) || (k == 1 && c == -1));
 c *= -1;
 }
}
Comment 2 bearophile_hugs 2010年07月25日 09:33:40 UTC
Normal compilation:
__Dmain	comdat
	assume	CS:__Dmain
L0:		enter	8,0
		mov	dword ptr -8[EBP],1
		mov	dword ptr -4[EBP],0
L12:		cmp	dword ptr -4[EBP],2
		jge	L42
		cmp	dword ptr -4[EBP],0
		jne	L24
		cmp	dword ptr -8[EBP],1
		je	L3A
L24:		cmp	dword ptr -4[EBP],1
		jne	L30
		cmp	dword ptr -8[EBP],0FFFFFFFFh
		je	L3A
L30:		mov	EAX,4
		call	near ptr _D4test8__assertFiZv
L3A:		neg	dword ptr -8[EBP]
		inc	dword ptr -4[EBP]
		jmp short	L12
L42:		xor	EAX,EAX
		leave
		ret
__Dmain	ends
_D4test8__assertFiZv	comdat
	assume	CS:_D4test8__assertFiZv
L0:		enter	4,0
		push	EAX
		mov	ECX,offset FLAT:_D4test12__ModuleInfoZ
		push	ECX
		call	near ptr __d_assertm
		leave
		ret
With -O:
__Dmain	comdat
	assume	CS:__Dmain
L0:		push	EAX
		push	EBX
		xor	EBX,EBX
		push	ESI
		mov	ESI,1
LA:		test	EBX,EBX
		je	L18
		mov	EAX,4
		call	near ptr _D4test8__assertFiZv
L18:		neg	ESI
		inc	EBX
		cmp	EBX,2
		jb	LA
		pop	ESI
		xor	EAX,EAX
		pop	EBX
		pop	ECX
		ret
__Dmain	ends
_D4test8__assertFiZv	comdat
	assume	CS:_D4test8__assertFiZv
L0:		push	EAX
		mov	ECX,offset FLAT:_D4test12__ModuleInfoZ
		push	EAX
		push	ECX
		call	near ptr __d_assertm
		pop	EAX
		ret
Comment 3 Don 2010年07月25日 11:06:14 UTC
Great work, bearophile! This worked on 2.032, fails on 2.034 and later. Fails on D1 as well. This bug goes to the top of the list.
Comment 4 Don 2010年07月25日 15:29:54 UTC
A one-line change in 2.034 which caused the regression. I think it's because
c *= -1; becomes NEGATE c, which isn't a binary operation. Here's a patch which undoes the change. I'm not sure which bug it was attempting to fix.
gother.c, line 480, inside chkprop().
 if (d->Eoper == OPasm) /* OPasm elems ruin everything */
 goto noprop;
- if (OTassign(d->Eoper) && EBIN(d)) // if assignment elem
+ if (OTassign(d->Eoper)) // if assignment elem
 { elem *t = Elvalue(d);
 if (t->Eoper == OPvar)
Comment 5 Walter Bright 2010年07月25日 22:39:40 UTC
http://www.dsource.org/projects/dmd/changeset/588 


AltStyle によって変換されたページ (->オリジナル) /