Jump to page: 1 2
Thread overview
[Issue 15629] [REG] wrong code with "-O -inline" but correct with "-O"
Jan 30, 2016
Ivan Kazmenko
Jan 30, 2016
Ivan Kazmenko
Feb 07, 2016
Kenji Hara
[Issue 15629] [REG2.066.0] wrong code with "-O -inline" but correct with "-O"
Feb 08, 2016
Kenji Hara
Feb 08, 2016
Kenji Hara
Apr 14, 2016
Walter Bright
[Issue 15629] [REG2.066.0] wrong code with '-O -inline' but correct with '-O'
Apr 15, 2016
Walter Bright
Apr 15, 2016
Walter Bright
January 30, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

--- Comment #1 from Ivan Kazmenko <gassa@mail.ru> ---
Here is the relevant disassembly of the object file produced 2.070.0:

-----
__Dmain PROC NEAR
;  COMDEF __Dmain
        sub     esp, 44                                 ; 0000 _ 83. EC, 2C
        mov     eax, 3                                  ; 0003 _ B8, 00000003
        mov     ecx, 1                                  ; 0008 _ B9, 00000001
        mov     dword ptr [esp+0CH], ebx                ; 000D _ 89. 5C 24, 0C
        mov     edx, offset FLAT:_D11TypeInfo_Ai6__initZ; 0011 _ BA,
00000000(segrel)
        mov     dword ptr [esp+10H], esi                ; 0016 _ 89. 74 24, 10
        mov     dword ptr [esp+18H], eax                ; 001A _ 89. 44 24, 18
        mov     dword ptr [esp+4H], ecx                 ; 001E _ 89. 4C 24, 04
        mov     dword ptr [esp], edx                    ; 0022 _ 89. 14 24
        call    __d_arrayliteralTX                      ; 0025 _ E8,
00000000(rel)
        mov     ebx, eax                                ; 002A _ 89. C3
        mov     eax, dword ptr [esp+18H]                ; 002C _ 8B. 44 24, 18
        mov     dword ptr [ebx], eax                    ; 0030 _ 89. 03
        mov     dword ptr [esp+20H], ebx                ; 0032 _ 89. 5C 24, 20
        mov     dword ptr [esp+1CH], 1                  ; 0036 _ C7. 44 24, 1C,
00000001
        mov     dword ptr [esp+4H], 1                   ; 003E _ C7. 44 24, 04,
00000001
        mov     dword ptr [esp], offset FLAT:_D11TypeInfo_Ai6__initZ; 0046 _
C7. 04 24, 00000000(segrel)
        call    __d_arrayliteralTX                      ; 004D _ E8,
00000000(rel)
        mov     esi, dword ptr [esp+20H]                ; 0052 _ 8B. 74 24, 20
        mov     edx, dword ptr [esp+14H]                ; 0056 _ 8B. 54 24, 14
        mov     dword ptr [eax], 9                      ; 005A _ C7. 00,
00000009
        mov     dword ptr [esp+28H], eax                ; 0060 _ 89. 44 24, 28
        mov     eax, dword ptr [esi]                    ; 0064 _ 8B. 06
        mov     ecx, eax                                ; 0066 _ 89. C1
        sar     ecx, 31                                 ; 0068 _ C1. F9, 1F
        xor     eax, ecx                                ; 006B _ 33. C1
        sub     eax, ecx                                ; 006D _ 2B. C1
        mov     dword ptr [esp+24H], 1                  ; 006F _ C7. 44 24, 24,
00000001
        cmp     edx, 3                                  ; 0077 _ 83. FA, 03
        jz      ?_0056                                  ; 007A _ 74, 2E
        push    eax                                     ; 007C _ 50
        mov     ecx, offset FLAT:?_0001                 ; 007D _ B9,
00000000(segrel)
        mov     ebx, 1                                  ; 0082 _ BB, 00000001
        push    ecx                                     ; 0087 _ 51
        push    ebx                                     ; 0088 _ 53
        push    dword ptr [esp+2CH]                     ; 0089 _ FF. 74 24, 2C
        push    dword ptr [esp+2CH]                     ; 008D _ FF. 74 24, 2C
        push    ecx                                     ; 0091 _ 51
        push    ebx                                     ; 0092 _ 53
        push    dword ptr [esp+44H]                     ; 0093 _ FF. 74 24, 44
        push    dword ptr [esp+44H]                     ; 0097 _ FF. 74 24, 44
        call
_D3std5stdio28__T7writelnTiTAyaTAiTAyaTAiZ7writelnFNfiAyaAiAyaAiZv; 009B _ E8,
00000000(rel)
        mov     eax, 12                                 ; 00A0 _ B8, 0000000C
        call    _D10testmodule8__assertFiZv             ; 00A5 _ E8,
00000000(rel)
?_0056: mov     ebx, dword ptr [esp+0CH]                ; 00AA _ 8B. 5C 24, 0C
        mov     esi, dword ptr [esp+10H]                ; 00AE _ 8B. 74 24, 10
        add     esp, 44                                 ; 00B2 _ 83. C4, 2C
        xor     eax, eax                                ; 00B5 _ 31. C0
        ret                                             ; 00B7 _ C3
__Dmain ENDP
-----

Looks like these lines are responsible:
-----
        mov     dword ptr [esp+18H], eax                ; 001A
        ...
        mov     edx, dword ptr [esp+14H]                ; 0056
        ...
        cmp     edx, 3                                  ; 0077
        jz      ?_0056                                  ; 007A
-----
I don't understand why the line at 0056 uses "dword ptr [esp+14H]" and not "dword ptr [esp+18H]".

(Disclaimer: I'm not fluent in asm.)

--
January 30, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

--- Comment #2 from Ivan Kazmenko <gassa@mail.ru> ---
Turned out that array b is also not necessary.

A bit less code (testmodule2.d):
-----
import std.math;
import std.stdio;

void main ()
{
    int [] a = [3];
    int value = abs (a[0]);
    if (a[0] != 3)
    {
        writeln (value, " ", a);
        assert (false);
    }
}
-----

Disassembly:
-----
__Dmain PROC NEAR
;  COMDEF __Dmain
        sub     esp, 36                                 ; 0000 _ 83. EC, 24
        mov     eax, 3                                  ; 0003 _ B8, 00000003
        mov     ecx, 1                                  ; 0008 _ B9, 00000001
        mov     dword ptr [esp+0CH], ebx                ; 000D _ 89. 5C 24, 0C
        mov     dword ptr [esp+10H], esi                ; 0011 _ 89. 74 24, 10
        mov     dword ptr [esp+18H], eax                ; 0015 _ 89. 44 24, 18
        mov     dword ptr [esp+4H], ecx                 ; 0019 _ 89. 4C 24, 04
        mov     dword ptr [esp], offset FLAT:_D11TypeInfo_Ai6__initZ; 001D _
C7. 04 24, 00000000(segrel)
        call    __d_arrayliteralTX                      ; 0024 _ E8,
00000000(rel)
        mov     edx, eax                                ; 0029 _ 89. C2
        mov     eax, dword ptr [esp+18H]                ; 002B _ 8B. 44 24, 18
        mov     dword ptr [esp+20H], edx                ; 002F _ 89. 54 24, 20
        mov     ebx, dword ptr [esp+20H]                ; 0033 _ 8B. 5C 24, 20
        mov     ecx, dword ptr [esp+14H]                ; 0037 _ 8B. 4C 24, 14
        mov     dword ptr [edx], eax                    ; 003B _ 89. 02
        mov     esi, dword ptr [ebx]                    ; 003D _ 8B. 33
        mov     eax, esi                                ; 003F _ 89. F0
        sar     eax, 31                                 ; 0041 _ C1. F8, 1F
        xor     esi, eax                                ; 0044 _ 33. F0
        sub     esi, eax                                ; 0046 _ 2B. F0
        mov     dword ptr [esp+1CH], 1                  ; 0048 _ C7. 44 24, 1C,
00000001
        cmp     ecx, 3                                  ; 0050 _ 83. F9, 03
        jz      ?_0056                                  ; 0053 _ 74, 24
        push    esi                                     ; 0055 _ 56
        mov     edx, offset FLAT:?_0001                 ; 0056 _ BA,
00000000(segrel)
        mov     ebx, 1                                  ; 005B _ BB, 00000001
        push    edx                                     ; 0060 _ 52
        push    ebx                                     ; 0061 _ 53
        push    dword ptr [esp+2CH]                     ; 0062 _ FF. 74 24, 2C
        push    dword ptr [esp+2CH]                     ; 0066 _ FF. 74 24, 2C
        call    _D3std5stdio21__T7writelnTiTAyaTAiZ7writelnFNfiAyaAiZv; 006A _
E8, 00000000(rel)
        mov     eax, 11                                 ; 006F _ B8, 0000000B
        call    _D11testmodule28__assertFiZv            ; 0074 _ E8,
00000000(rel)
?_0056: mov     ebx, dword ptr [esp+0CH]                ; 0079 _ 8B. 5C 24, 0C
        mov     esi, dword ptr [esp+10H]                ; 007D _ 8B. 74 24, 10
        add     esp, 36                                 ; 0081 _ 83. C4, 24
        xor     eax, eax                                ; 0084 _ 31. C0
        ret                                             ; 0086 _ C3
__Dmain ENDP
-----

Now the lines supposed to carry the "3" seem to be
-----
        mov     eax, 3                                  ; 0003 _ B8, 00000003
        mov     dword ptr [esp+18H], eax                ; 0015 _ 89. 44 24, 18
        mov     eax, dword ptr [esp+18H]                ; 002B _ 8B. 44 24, 18
        mov     dword ptr [edx], eax                    ; 003B _ 89. 02
-----
and then for some reason
-----
        mov     ecx, dword ptr [esp+14H]                ; 0037 _ 8B. 4C 24, 14
        cmp     ecx, 3                                  ; 0050 _ 83. F9, 03
        jz      ?_0056                                  ; 0053 _ 74, 24
-----

--
February 07, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

--- Comment #3 from Kenji Hara <k.hara.pg@gmail.com> ---
Dustmited case code:

void main()
{
    int[] a = [3];
    int value = abs(a[0]);
    assert(a[0] == 3);
    writeln(value, " ", a);
}

Num abs(Num)(Num x)
{
    return x >= 0 ? x : -x;
}

struct File
{
    struct LockingTextWriter
    {
        this(File) {}

        ~this() @trusted {}
    }

    auto lockingTextWriter()
    {
        return LockingTextWriter();
    }
}

File stdout;

void writeln(T...)(T args)
{
    stdout.lockingTextWriter;
}

--
February 08, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[REG] wrong code with "-O   |[REG2.066.0] wrong code
                   |-inline" but correct with   |with "-O -inline" but
                   |"-O"                        |correct with "-O"

--- Comment #4 from Kenji Hara <k.hara.pg@gmail.com> ---
(In reply to Kenji Hara from comment #3)
> Dustmited case code:

Sorry, the reduced code cannot reproduce exactly same regression with the
original.
I'll open one more issue for that.

Correct minimized code is:

void main()
{
    int[] a = [3];
    int value = abs(a[0]);
    assert(a[0] == 3);
    writeln(value, " ", a);
}

Num abs(Num)(Num x)
{
    return x >= 0 ? x : -x;
}

struct File
{
    struct LockingTextWriter
    {
        this(File) {}

        ~this() @trusted {}
    }

    auto lockingTextWriter()
    {
        return LockingTextWriter();
    }
}

File stdout;

void writeln(T...)(T args)
{
    stdout.lockingTextWriter();
}

Introduced in:
https://github.com/D-Programming-Language/dmd/pull/3620
and its fixup PR:
https://github.com/D-Programming-Language/dmd/pull/3656

So this is a regression from 2.066.0.

--
February 08, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> ---
(In reply to Kenji Hara from comment #4)
> Sorry, the reduced code cannot reproduce exactly same regression with the
> original.
> I'll open one more issue for that.

Ah, I got understanding. The reduced case in comment#3 has generated wrong code with -inline, since the change by PR#4474: https://github.com/D-Programming-Language/dmd/pull/4474

However, comment#3 and comment#4 are essentially same. The only one difference is the parenthesis on stdout.lockingTextWriter(); in writeln template function. So, this wrong-code issue in the comment#3 case was hidden by issue 14264, and it's *fixed* by PR#4474. Therefore the two cases can hit this issue with current master.

--
April 14, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com

--- Comment #6 from Walter Bright <bugzilla@digitalmars.com> ---
Comment 3 further reduces to:

 void main() {
    int[] a = [3];
    int value = a[0] >= 0 ? a[0] : -a[0];
    assert(a[0] == 3);
    writeln(value, a);
 }

void writeln(int v, int[] a) {
 }

and only -O is needed (not -inline).

--
April 15, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[REG2.066.0] wrong code     |[REG2.066.0] wrong code
                   |with "-O -inline" but       |with '-O -inline' but
                   |correct with "-O"           |correct with '-O'

--
April 15, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Hardware|x86                         |All
                 OS|Windows                     |All

--- Comment #7 from Walter Bright <bugzilla@digitalmars.com> ---
https://github.com/D-Programming-Language/dmd/pull/5666

This is a serious bug.

--
April 15, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

--- Comment #8 from github-bugzilla@puremagic.com ---
Commits pushed to stable at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/6b54b4bcf6dfaa343992f3524e1bf15ffca07da3
fix Issue 15629 - [REG2.066.0] wrong code with '-O -inline' but correct with
'-O'

https://github.com/D-Programming-Language/dmd/commit/b0d60736a583e22fc685cbc2b7f436e520073f91 Merge pull request #5666 from WalterBright/fix15629

fix Issue 15629 - [REG2.066.0] wrong code with '-O -inline' but correct with '-O'

--
April 15, 2016
https://issues.dlang.org/show_bug.cgi?id=15629

github-bugzilla@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--
« First   ‹ Prev
1 2