Bug 17676 – [REG 2.075] bad inlining of functions with multiple return statements
Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Windows
Creation time
2017-07-23T14:01:00Z
Last change time
2017-08-09T15:04:47Z
Assigned to
nobody
Creator
r.sagitario
Comments
Comment #0 by r.sagitario — 2017-07-23T14:01:14Z
import core.stdc.stdio;
__gshared bool bgEnable = 1;
void smallAlloc() nothrow
{
fullcollect();
}
size_t fullcollect() nothrow
{
if(bgEnable)
return fullcollectTrigger();
return fullcollectNow();
}
size_t fullcollectNow() nothrow
{
pragma(inline, false);
printf("fullcollectNow\n");
return 1;
}
size_t fullcollectTrigger() nothrow
{
pragma(inline, false);
printf("fullcollectTrigger\n");
return 0;
}
void main()
{
smallAlloc();
}
Without inlining, this just prints "fullcollectTrigger", while compiling with -inline causes both messages to be printed.
The assembly of smallAlloc looks like this (dmd -O -inline -release):
_D7reg681510smallAllocFNbZv:
0000000000000000: 55 push rbp
0000000000000001: 48 8B EC mov rbp,rsp
0000000000000004: 48 83 EC 20 sub rsp,20h
0000000000000008: 40 80 3D 00 00 00 cmp byte ptr [_D7reg68158bgEnableb],0
00 00
0000000000000010: 74 05 je 0000000000000017
0000000000000012: E8 00 00 00 00 call _D7reg681518fullcollectTriggerFNbZm
0000000000000017: E8 00 00 00 00 call _D7reg681514fullcollectNowFNbZm
000000000000001C: 48 8B E5 mov rsp,rbp
000000000000001F: 5D pop rbp
0000000000000020: C3 ret
Note the missing jump between the two calls. If an "else" is inserted between the two return statements in fullCollect(), it works correctly.
This doesn't happen with dmd 2.074. Introduced by https://github.com/dlang/dmd/pull/6815
Comment #1 by uplink.coder — 2017-07-23T15:03:58Z
please consider adding the -vcg-ast output for such issues in the future.
It is there to help debugging inliner issues amongst other things :)
The -vcg-ast output shows that the return is indeed removed:
import object;
import core.stdc.stdio;
__gshared bool bgEnable = true;
nothrow void smallAlloc()
{
{
if (bgEnable)
fullcollectTrigger();
fullcollectNow();
}
}
nothrow ulong fullcollect()
{
if (bgEnable)
return fullcollectTrigger();
return fullcollectNow();
}
nothrow ulong fullcollectNow()
{
printf("fullcollectNow\x0a");
return 1LU;
}
nothrow ulong fullcollectTrigger()
{
printf("fullcollectTrigger\x0a");
return 0LU;
}
void main()
{
{
{
if (bgEnable)
fullcollectTrigger();
fullcollectNow();
}
}
return 0;
}
Comment #2 by dlang-bugzilla — 2017-07-23T15:06:36Z
(In reply to uplink.coder from comment #1)
> please consider adding the -vcg-ast output for such issues in the future.
Why? If it can be trivially obtained by anyone, and unless it's part of the description of the bug, it's just junk. Makes as much sense as including compiled object files or executables.
Comment #3 by uplink.coder — 2017-07-23T15:33:21Z
It helps me to spot where the stuff is going wrong.
If it's the codegen messing up or wether the inliner is indeed at fault.
it's more readable then the asm.
Comment #4 by dlang-bugzilla — 2017-07-23T17:07:36Z
If the source code is provided then you can just get any intermediary or debug outputs yourself. Let's please not clutter Bugzilla with junk.
Comment #5 by greeenify — 2017-07-23T17:19:52Z
> Let's please not clutter Bugzilla with junk.
FWIW the flag is _not_ still not documented ;-)