Bug 4298 – Constant array translated to unnecessary array literal creation

Status
RESOLVED
Resolution
DUPLICATE
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
x86
OS
All
Creation time
2010-06-08T17:57:00Z
Last change time
2014-02-15T02:43:58Z
Keywords
patch, performance
Assigned to
nobody
Creator
rsinfu
Depends on
3779
Blocks
4297

Comments

Comment #0 by rsinfu — 2010-06-08T17:57:52Z
Using constant (const or immutable) dynamic array causes unnecessary array literal creation if the constant array has an initializer. This behavior degrades performance, and also causes the bug 4297. Test code and the disassembled output: -------------------- immutable int[] TABLE = [ 1, 2, 3 ]; void foo() { const(int)[] tab = TABLE; } -------------------- _D4test3fooFZv: push EBP mov EBP,ESP push EBX push 3 push 2 push 1 push 3 mov EAX,offset FLAT:_D12TypeInfo_Axi6__initZ@SYM32 push EAX call _d_arrayliteralT@PC32 mov ECX,EAX mov EBX,3 add ESP,014h pop EBX pop EBP ret --------------------
Comment #1 by rsinfu — 2010-06-08T18:02:25Z
Patch against DMD r526: ==================== --- src/optimize.c +++ src/optimize.c @@ -58,10 +58,9 @@ Expression *expandVar(int result, VarDeclaration *v) return e; } - Type *tb = v->type->toBasetype(); if (result & WANTinterpret || v->storage_class & STCmanifest || - (tb->ty != Tsarray && tb->ty != Tstruct) + v->type->isscalar() ) { if (v->init) ==================== The if condition is changed so that only scalar constants are expanded. Affected types are Tarray, Taarray, Tclass and Tdelegate. Arrays and associative arrays should not be expanded (the reported problem); classes and delegates are not necessary to be expanded.
Comment #2 by rsinfu — 2010-06-15T15:15:15Z
(In reply to comment #1) With the patch in comment #1, this example does not link: -------------------- void main() { immutable s = "abc"; auto t = s[0 .. $ - 1]; } -------------------- % dmd -run test test.o(.text._Dmain+0x1a): In function `_Dmain': : undefined reference to `_D4test4mainFZv8__dollarxk' -------------------- The link error does not occur if the dollar $ is replaced with s.length. The real cause of the link error may be elsewhere (only $ causes a link error?), but the patch itself doesn't work anyway. So I remove the 'patch' keyword.
Comment #3 by rsinfu — 2010-06-15T16:26:14Z
(In reply to comment #2) > The link error does not occur if the dollar $ is replaced with s.length. > > The real cause of the link error may be elsewhere (only $ causes a link > error?), but the patch itself doesn't work anyway. So I remove the 'patch' > keyword. The linker error about __dollar is caused by bug 3779. I think the patch in comment #1 is valid if that bug is fixed. Restored the patch keyword and added a dependency to bug 3779.
Comment #4 by dsimcha — 2010-07-25T10:09:58Z
*** Issue 3370 has been marked as a duplicate of this issue. ***
Comment #5 by dsimcha — 2010-09-08T20:11:40Z
*** Issue 4631 has been marked as a duplicate of this issue. ***
Comment #6 by yebblies — 2012-02-01T19:27:49Z
*** This issue has been marked as a duplicate of issue 2356 ***