Bug 2008 – Poor optimization of functions with ref parameters

Functions with ref parameters can be up to 2x slower than functions which parameters are not passed by reference. It is especially visible for programs which are compiled with all optimizations on: -O -inline -release Test program: ------------------------ char peek_ref(ref char[] a) { return a[0]; } char peek_noref(char[] a) { return a[0]; } ulong rdtsc() { asm { naked; rdtsc; ret; } } void main() { char[] tst = "test string"; char c; long starttime; writef("Direct: "); starttime = rdtsc; for(int i=0; i<1000; i++) c = tst[0]; writefln(c, " - time: ", rdtsc - starttime); writef("With ref: "); starttime = rdtsc; for(int i=0; i<1000; i++) c = tst.peek_ref(); writefln(c, " - time: ", rdtsc - starttime); writef("Without ref: "); starttime = rdtsc; for(int i=0; i<1000; i++) c = tst.peek_noref(); writefln(c, " - time: ", rdtsc - starttime); } ----------------------

I believe the problem is that ref parameters disable inlining in DMD. Or so I think I heard somewhere before.

This is true, and also applies to D2. Apparently ref prevents inlining. It's somewhere in the comments in inline.c. Why, I don't know. The weird thing is that apparently DMD inlines stuff with pointer parameters, and ref parameters are just syntactic sugar for pointers. Here's a test program and the disassembly. import std.stdio; // Shouldn't this generate *exactly* the same code as ptrSwap? void swap(T)(ref T a, ref T b) { T temp = a; a = b; b = temp; } void ptrSwap(T)(T* a, T* b) { T temp = *a; *a = *b; *b = temp; } void main() { uint a, b; swap(a, b); ptrSwap(&a, &b); writeln(a); // Keep DMD from optimizing out ptrSwap entirely. } Here's the disassembly of the relevant portion: COMDEF __Dmain push eax ; push eax ; xor eax, eax ; push ebx ; lea ecx, [esp+4H] ; mov dword ptr [esp+4H], eax ; mov dword ptr [esp+8H], eax ; push ecx ; lea eax, [esp+0CH] ; call _D5test711__T4swapTkZ4swapFKkKkZv ; mov edx, dword ptr [esp+4H] ; mov ebx, dword ptr [esp+8H] ; mov dword ptr [esp+4H], ebx ; mov eax, offset FLAT:_main ; mov dword ptr [esp+8H], edx ; push ebx ; push 10 ; call _D3std5stdio4File14__T5writeTkTaZ5writeMFkaZv; xor eax, eax ; pop ebx ; add esp, 8 ; ret ; __Dmain ENDP

Recent change to dmd forced everybody to use opEquals with ref arguments. That leaves no other option than the poorly optimised one. Isn't it time to fix this bug?

(In reply to comment #3) > Recent change to dmd forced everybody to use opEquals with ref arguments. That > leaves no other option than the poorly optimised one. Isn't it time to fix this > bug? I think the more important fix is to start allowing opEquals with non-ref arguments again. Only allowing ref arguments is ridiculous because it forces the argument to be an lvalue, thus creating a horribly ugly inconsistency with builtin types.

(In reply to comment #4) > (In reply to comment #3) > > Recent change to dmd forced everybody to use opEquals with ref arguments. That > > leaves no other option than the poorly optimised one. Isn't it time to fix this > > bug? > > I think the more important fix is to start allowing opEquals with non-ref > arguments again. Only allowing ref arguments is ridiculous because it forces > the argument to be an lvalue, thus creating a horribly ugly inconsistency with > builtin types. That's true. Perhaps functions with signature (const ref T) could accept rvalues as well, as the semantics is equivalent to (T)?

Created attachment 626 a potential patch (lots of debugging code left in for now) This diff likely needs to be used in conjunction with the patch in bug 4149. I haven't tested it separately to find out. I also haven't tested it with more than the simple test case in bug 4149.

Testing with the code from Marcin, using -O -inline and fixing several bugs due to changes since 2008: Direct: t - time: 4319 With ref: t - time: 4319 Without ref: t - time: 4382 The code changes (not perfect, but enough to get it to compile): 1) add .dup to the "test string" literal 2) s/writefln/writeln/

Testing with David's code, with -O -inline, here's the new _Dmain: _Dmain: push EBP mov EBP,ESP mov EAX,offset FLAT:_D3std5stdio6stdoutS3std5stdio4File@SYM32 push 0 push 0Ah call near ptr _D3std5stdio4File14__T5writeTkTaZ5writeMFkaZv@PC32 xor EAX,EAX pop EBP ret Without -O: _Dmain: push EBP mov EBP,ESP sub ESP,024h push EBX push ESI xor EAX,EAX mov -024h[EBP],EAX mov -020h[EBP],EAX lea ECX,-024h[EBP] mov -01Ch[EBP],ECX lea EDX,-020h[EBP] mov -018h[EBP],EDX mov EBX,[ECX] mov -014h[EBP],EBX mov ESI,[EDX] mov [ECX],ESI mov [EDX],EBX lea EAX,-024h[EBP] mov -010h[EBP],EAX lea ECX,-020h[EBP] mov -0Ch[EBP],ECX mov EDX,[EAX] mov -8[EBP],EDX mov EBX,[ECX] mov [EAX],EBX mov [ECX],EDX mov ESI,-024h[EBP] mov -4[EBP],ESI push ESI push 0Ah mov EAX,offset FLAT:_D3std5stdio6stdoutS3std5stdio4File@SYM32 call near ptr _D3std5stdio4File14__T5writeTkTaZ5writeMFkaZv@PC32 xor EAX,EAX pop ESI pop EBX leave ret

Created attachment 627 Enable inlining of functions with ref parameters, v2 The previous patch didn't pass the dmd test suite (still need to understand why). This newer patch isolates the changes to the inlining code to generate AST closer to what would be manually written for hand inlined code. I also dropped all of the debugging code and unrelated code cleanup. This version DOES pass the dmd test suite. I've also confirmed that it is NOT dependent on the patch attached to bug 4149.

Created attachment 633 add support for out parameters as well (so both ref and out) The previous patch only added support for ref parameters. This updated patch also includes adding support for inlining of functions that have out parameters.

changeset 497. Does static arrays too.