https://d.puremagic.com/issues/show_bug.cgi?id=12164

           Summary: Function returning ptrdiff_t.min in 64-bit returning 0
                    when -O is set.
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: wrong-code
          Severity: major
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: opantm2+dbugs@gmail.com


--- Comment #0 from Kapps <opantm2+dbugs@gmail.com> 2014-02-14 17:58:07 PST ---
The title isn't very descriptive, but perhaps the sample is more useful.
Reduced from std.variant.
Uncommenting the writeln, using -m32, commenting out the if(*rhsPA == *zis)
return 0, or not passing in -O causes the problem to not occur. Otherwise
compile the below with -O -m64 to have it occur.

Sample:

import std.stdio, std.exception, std.c.string;

struct Foo {

    ubyte[32] store;

    this(A value) {
        memcpy(&store, &value, value.sizeof);
    }

    static ptrdiff_t compare(A* rhsPA, A* zis) {
        if (*rhsPA == *zis)
            return 0;
        //writeln("Returning min");
        return ptrdiff_t.min;
    }

    bool opEquals(Foo rhs) const {
        auto zis = cast(A*)&store;
        auto rhsPA = cast(A*)&rhs.store;
        // Below prints 0 if previous writeln line is commented and -O is set.
        // Otherwise prints -9223372036854775808 as expected.
        writeln(compare(rhsPA, zis));
        return compare(rhsPA, zis) == 0;
    }
}

static struct A { int a; }

void main() {
    enforce(Foo(A(3)) != Foo(A(4)));
}

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies@gmail.com


--- Comment #1 from yebblies <yebblies@gmail.com> 2014-02-16 03:05:08 EST ---
Wrong code, yummy!

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164


Kapps <opantm2+dbugs@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |critical


--- Comment #2 from Kapps <opantm2+dbugs@gmail.com> 2014-02-16 00:02:09 PST ---
Raised priority to critical as it's a difficult to notice or track down (since trying to observe it with something like a writeln or setting a global variable causes it to no longer occur) bug that will silently result in different behaviour only in release mode.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164



--- Comment #3 from yebblies <yebblies@gmail.com> 2014-03-07 18:49:05 EST ---
Reduced:

ptrdiff_t compare(A* rhsPA, A* zis)
{
    if (*rhsPA == *zis)
        return 0;
    return ptrdiff_t.min;
}

struct A
{
    int a;
}

void main()
{
    auto a = A(3);
    auto b = A(4);
    assert(!compare(&a, &b));
}

The backend tried to emit OPmemcmp and screws it up somehow, probably missing a REX_W prefix or two.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164


safety0ff.bugz <safety0ff.bugz@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |safety0ff.bugz@gmail.com


--- Comment #4 from safety0ff.bugz <safety0ff.bugz@gmail.com> 2014-03-09 22:08:33 PDT ---
(In reply to comment #3)
> The backend tried to emit OPmemcmp and screws it up somehow, probably missing a REX_W prefix or two.

When I looked at the assembly on my end the memcmp looked fine.
The emitted code for returning ptrdiff.min or 0 depending on the memcmp result
looks like it has more than one issue in it.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164



--- Comment #5 from github-bugzilla@puremagic.com 2014-03-15 10:21:55 PDT ---
Commit pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/6b91d4e1a1d385cf961ff6b68848bf3e0ca93245 Added workaround for bug 12164.

Style: Fix lack of space after if.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164



--- Comment #6 from yebblies <yebblies@gmail.com> 2014-03-16 04:26:30 EST ---
(In reply to comment #4)
> (In reply to comment #3)
> > The backend tried to emit OPmemcmp and screws it up somehow, probably missing a REX_W prefix or two.
> 
> When I looked at the assembly on my end the memcmp looked fine.
> The emitted code for returning ptrdiff.min or 0 depending on the memcmp result
> looks like it has more than one issue in it.

If someone can identify exactly which instructions are wrong, and what they should be, I can probably fix it fairly easily.  Otherwise it will have to wait, I don't have the energy to go digging through the assembly right now.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164



--- Comment #7 from safety0ff.bugz <safety0ff.bugz@gmail.com> 2014-03-31 07:33:59 PDT ---
(In reply to comment #6)
> If someone can identify exactly which instructions are wrong, and what they should be, I can probably fix it fairly easily.  Otherwise it will have to wait, I don't have the energy to go digging through the assembly right now.

I don't think the solution is as simple as you think (it's not a simple REX
issue):
Un-optimized (correct) assembly:
    55                    push    RBP
    48 8B EC              mov    RBP,RSP
    48 83 EC 10           sub    RSP,010h
    48 B9 04 00 00 00 00 00 00 00     mov    RCX,4
    33 C0                 xor    EAX,EAX
    F3                    rep
    A6                    cmpsb
    75 05                 jne    L1D
    48 31 C0              xor    RAX,RAX
    C9                    leave
    C3                    ret
L1D:    48 B8 00 00 00 00 00 00 00 80     mov    RAX,08000000000000000h
    C9                    leave
    C3                    ret

Incorrect -O assembly:
    55                    push    RBP
    48 8B EC              mov    RBP,RSP
    48 83 EC 10           sub    RSP,010h
    48 B9 04 00 00 00 00 00 00 00     mov    RCX,4
    33 C0                 xor    EAX,EAX
    F3                    rep
    A6                    cmpsb
    74 05                 je    L1D
    1B C0                 sbb    EAX,EAX
    83 D8 FF              sbb    EAX,0FFFFFFFFFFFFFFFFh
L1D:    83 F8 01              cmp    EAX,1
    19 C0                 sbb    EAX,EAX
    31 C0                 xor    EAX,EAX
    48 8B E5              mov    RSP,RBP
    5D                    pop    RBP
    C3                    ret

As you can see, it invariantly returns zero after doing the memcmp.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164


Orvid King <blah38621@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |blah38621@gmail.com


--- Comment #8 from Orvid King <blah38621@gmail.com> 2014-03-31 07:49:58 PDT ---
One thing I am curious of in that generated code, why is it allocating 16 bytes on the stack when it doesn't use the stack? (sub RSP, 0x10) Also, I missing something here, but when did parameters start getting passed in ESI and EDI?

As a bonus question, I have to ask, why are we doing that comparison with cmpsb when the size of the values being compared is known to be a multiple of 4, in which case, shouldn't we be using cmpsd? (this is quite literally a 4x speed improvement, as both instructions have the same cycle count)

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

https://d.puremagic.com/issues/show_bug.cgi?id=12164



--- Comment #9 from Orvid King <blah38621@gmail.com> 2014-03-31 08:49:18 PDT ---
(In reply to comment #8)
> One thing I am curious of in that generated code, why is it allocating 16 bytes on the stack when it doesn't use the stack? (sub RSP, 0x10) Also, I missing something here, but when did parameters start getting passed in ESI and EDI?
> 
> As a bonus question, I have to ask, why are we doing that comparison with cmpsb when the size of the values being compared is known to be a multiple of 4, in which case, shouldn't we be using cmpsd? (this is quite literally a 4x speed improvement, as both instructions have the same cycle count)

I seem to have forgotten about cmpsq, which is available on x64, for the same cycle count, which would be 8x faster.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------