Bug 20189 – Codegen - functions that call other functions with the same arguments do redundant copying between stack and registers.

Status
NEW
Severity
enhancement
Priority
P4
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
All
Creation time
2019-09-03T02:12:45Z
Last change time
2024-12-13T19:05:25Z
Assigned to
No Owner
Creator
elronnd
Moved to GitHub: dmd#19618 →

Comments

Comment #0 by elronnd — 2019-09-03T02:12:45Z
Consider this code - contrived, but representative of an actual problem I had: pragma(inline, false) int second(int a, int b) { return a+b; } pragma(inline, false) int first(int a, int b) { return second(a, b); } It generates this code for first() with dmd version 2.087.1, with optimizations enabled: push rbp mov rbp,rsp sub rsp,0x10 mov DWORD PTR [rbp-0x10],edi mov DWORD PTR [rbp-0x8],esi mov esi,DWORD PTR [rbp-0x8] mov edi,DWORD PTR [rbp-0x10] call 3c66c <int perftest.second(int, int)> mov rsp,rbp pop rbp ret It moves both of its parameters--which are in the edi and esi registers--onto the stack, and then...back into the registers. Those 4 movs in the middle could be elided completely. (As a further optimization, 'return func(...)' could be translated into 'jmp func', instead of 'call func; ret'--ldc does this.)
Comment #1 by elronnd — 2019-09-03T02:18:41Z
Ideally, a fix would also handle partial or complete argument reordering -- so if first() called second(b, a), then all the stack munging could just be straight 'xchg esi, edi'. (However, making that work in more complicated functions that pass many arguments--some from their own arguments, others from local variables--would probably require a lot more effort, and mess with the register allocator quite a bit: is it worth it to place this local variable in a register instead of on the stack, knowing that means clobbering that register so it will have to be restored later?)
Comment #2 by robert.schadek — 2024-12-13T19:05:25Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19618 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB