http://d.puremagic.com/issues/show_bug.cgi?id=1149

           Summary: Optimizer: obsolete array length loads, common subexpr.
                    elimin. not working
           Product: D
           Version: 1.012
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: jascha@mainia.de


consider the following code:

void main()
{
    uint[] arr;
    arr.length = 4;
    arr[0] = 1;
    arr[1] = 1;
    arr[2] = 1;
    arr[3] = 1;
}

for the 4 assignments, "dmd -O -inline" generates

mov     ECX,1
mov     EDX,010h[ESP]
mov     EAX,0Ch[ESP]
mov     [EDX],ECX
mov     EDX,010h[ESP]
mov     EAX,0Ch[ESP]
mov     4[EDX],ECX
mov     EDX,010h[ESP]
mov     EAX,0Ch[ESP]
mov     8[EDX],ECX
mov     EDX,010h[ESP]
mov     EAX,0Ch[ESP]
mov     0Ch[EDX],ECX

instead of

mov     ECX,1
mov     EDX,010h[ESP]
mov     [EDX],ECX
mov     4[EDX],ECX
mov     8[EDX],ECX
mov     0Ch[EDX],ECX

- the length of the array is always loaded, even if it's not used - the array pointer is loaded multiple times into the same register

the common subexpression elimination appears to be working for the value in ECX, but not for the pointer in EDX.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1149


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull
                 CC|                            |yebblies@gmail.com
            Version|1.012                       |D1 & D2
         AssignedTo|nobody@puremagic.com        |yebblies@gmail.com


--- Comment #1 from yebblies <yebblies@gmail.com> 2012-02-21 02:25:01 EST ---
https://github.com/D-Programming-Language/dmd/pull/749

This makes the code generated for arrays identical to the code for pointers.

The pointer is still reloaded before every assignment.

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #2 from github-bugzilla@puremagic.com 2012-02-22 14:22:53 PST ---
Commit pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/a999486f9bb0b6ca2eeff1964b8f9c90b8841867 Merge pull request #749 from yebblies/issue1149

Improve codegen for OPmsw

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149


bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #3 from bearophile_hugs@eml.cc 2012-02-22 15:38:35 PST ---
With the test code, this is the X86 asm I was seeing before this patch (-O
-release -inline):


__Dmain comdat
L0:     sub ESP,0Ch
        lea EAX,[ESP]
        mov ECX,offset FLAT:_D11TypeInfo_Ak6__initZ
        mov [ESP],0
        mov dword ptr 4[ESP],0
        push    EAX
        push    4
        push    ECX
        call    near ptr __d_arraysetlengthT
        mov ECX,010h[ESP]
        mov EDX,1
        mov EAX,0Ch[ESP]
        mov [ECX],EDX
        mov ECX,010h[ESP]
        mov EAX,0Ch[ESP]
        mov 4[ECX],EDX
        mov ECX,010h[ESP]
        mov EAX,0Ch[ESP]
        mov 8[ECX],EDX
        mov ECX,010h[ESP]
        mov EAX,0Ch[ESP]
        mov 0Ch[ECX],EDX
        add ESP,0Ch
        add ESP,0Ch
        xor EAX,EAX
        ret

-----------------------------

After the patch:

__Dmain comdat
L0:     sub ESP,0Ch
        lea EAX,[ESP]
        mov ECX,offset FLAT:_D11TypeInfo_Ak6__initZ
        mov [ESP],0
        mov dword ptr 4[ESP],0
        push    EAX
        push    4
        push    ECX
        call    near ptr __d_arraysetlengthT
        mov ECX,010h[ESP]
        mov EDX,1
        mov EAX,0Ch[ESP]
        mov [ECX],EDX
        mov ECX,010h[ESP]
        mov EAX,0Ch[ESP]
        mov 4[ECX],EDX
        mov ECX,010h[ESP]
        mov EAX,0Ch[ESP]
        mov 8[ECX],EDX
        mov ECX,010h[ESP]
        mov EAX,0Ch[ESP]
        mov 0Ch[ECX],EDX
        add ESP,0Ch
        add ESP,0Ch
        xor EAX,EAX
        ret

--------------------------------

objectdump after the patch, on a 64 bit systems (courtesy of q66 on IRC #D),
compiled with -release:

0000000000000000 <_Dmain>:
   0:   55                      push   %rbp
   1:   48 8b ec                mov    %rsp,%rbp
   4:   48 83 ec 18             sub    $0x18,%rsp
   8:   53                      push   %rbx
   9:   48 c7 45 f0 00 00 00    movq   $0x0,-0x10(%rbp)
  10:   00
  11:   48 c7 45 f8 00 00 00    movq   $0x0,-0x8(%rbp)
  18:   00
  19:   48 8d 55 f0             lea    -0x10(%rbp),%rdx
  1d:   48 be 04 00 00 00 00    mov    $0x4,%rsi
  24:   00 00 00
  27:   48 bf 00 00 00 00 00    mov    $0x0,%rdi
  2e:   00 00 00
  31:   e8 00 00 00 00          callq  36 <_Dmain+0x36>
  36:   b8 01 00 00 00          mov    $0x1,%eax
  3b:   48 8b 55 f8             mov    -0x8(%rbp),%rdx
  3f:   48 8b 5d f0             mov    -0x10(%rbp),%rbx
  43:   89 02                   mov    %eax,(%rdx)
  45:   48 8b 55 f8             mov    -0x8(%rbp),%rdx
  49:   48 8b 5d f0             mov    -0x10(%rbp),%rbx
  4d:   89 42 04                mov    %eax,0x4(%rdx)
  50:   48 8b 55 f8             mov    -0x8(%rbp),%rdx
  54:   48 8b 5d f0             mov    -0x10(%rbp),%rbx
  58:   89 42 08                mov    %eax,0x8(%rdx)
  5b:   48 8b 55 f8             mov    -0x8(%rbp),%rdx
  5f:   48 8b 5d f0             mov    -0x10(%rbp),%rbx
  63:   89 42 0c                mov    %eax,0xc(%rdx)
  66:   31 c0                   xor    %eax,%eax
  68:   5b                      pop    %rbx
  69:   c9                      leaveq
  6a:   c3                      retq

--------------------------------

So is this patch working?

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #4 from yebblies <yebblies@gmail.com> 2012-02-23 14:20:18 EST ---
(In reply to comment #3)
> With the test code, this is the X86 asm I was seeing before this patch (-O
> -release -inline):
> 
[snip]
> 
> So is this patch working?

Oops.  I tried to disable it for floating point and ended up disabling it for all non-integers.

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #5 from bearophile_hugs@eml.cc 2012-02-22 20:00:29 PST ---
(In reply to comment #4)

> Oops.  I tried to disable it for floating point and ended up disabling it for all non-integers.

I have tried it with this code:

void main() {
    int[] arr;
    arr.length = 4;
    arr[0] = 1;
    arr[1] = 1;
    arr[2] = 1;
    arr[3] = 1;
}


And I am seeing something similar to the uint[] case.

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #6 from yebblies <yebblies@gmail.com> 2012-02-23 15:20:57 EST ---
(In reply to comment #5)
> (In reply to comment #4)
> 
> > Oops.  I tried to disable it for floating point and ended up disabling it for all non-integers.
> 
> I have tried it with this code:
> 
> void main() {
>     int[] arr;
>     arr.length = 4;
>     arr[0] = 1;
>     arr[1] = 1;
>     arr[2] = 1;
>     arr[3] = 1;
> }
> 
> 
> And I am seeing something similar to the uint[] case.

Yeah, the optimization in my patch is effectively disabled.

I changed it to integer-only because it was crashing in some floating point code - but the most significant word of an array isn't an integer, it's a pointer.

I've tracked to codegen down, it seems like 'fnstsw  ax' destroys eax and it never gets restored.  Without the optimization loading the pointer into eax never gets picked up as a cse and removed in the first place.

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #7 from github-bugzilla@puremagic.com 2012-02-23 00:05:21 PST ---
Commit pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/1d1bc367c131144e764d9fb81c36651407e3337d Merge pull request #759 from yebblies/issue1149

Fix OPmsw codegen

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #8 from bearophile_hugs@eml.cc 2012-02-23 04:31:27 PST ---
Better:

__Dmain comdat
L0:     sub ESP,0Ch
        mov ECX,offset FLAT:_D11TypeInfo_Ak6__initZ
        push    EBX
        push    ESI
        lea EAX,8[ESP]
        mov dword ptr 8[ESP],0
        mov dword ptr 0Ch[ESP],0
        push    EAX
        push    4
        push    ECX
        call    near ptr __d_arraysetlengthT
        mov EBX,018h[ESP]
        mov EDX,1
        mov [EBX],EDX
        mov ESI,018h[ESP]
        mov 4[ESI],EDX
        mov EAX,018h[ESP]
        mov 8[EAX],EDX
        mov ECX,018h[ESP]
        xor EAX,EAX
        mov 0Ch[ECX],EDX
        add ESP,0Ch
        pop ESI
        pop EBX
        add ESP,0Ch
        ret

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

http://d.puremagic.com/issues/show_bug.cgi?id=1149



--- Comment #9 from yebblies <yebblies@gmail.com> 2012-02-24 00:20:28 EST ---
(In reply to comment #8)
> Better:
> 

Yeah.  I have no idea why it's using so many registers though.

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