Optimizer fails to remove comparison loop when comparing array against null

[dlangBugzillaToGithub]

yebblies (@yebblies) reported this on 2015-04-10T12:21:27Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=14436

CC List

• Marc Schütz

Description

The comparison in this code

int[] arr;
void main()
{
    if (arr != null)
    {
        arr = [1];
    }
}

gets lowered to

!arr.length && !memcmp(arr.ptr, null, arr.length)

The optimizer should be able to tell that the memcmp would always return 0 and skip the check.

[dlangBugzillaToGithub]

yebblies commented on 2015-04-10T15:28:51Z

https://github.com/D-Programming-Language/dmd/pull/4574

[dlangBugzillaToGithub]

github-bugzilla commented on 2015-04-12T21:32:45Z

Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/7660158f16b9c988d8ff16592ee442051ea95ffa
Fix Issue 14436 - Optimizer fails to remove comparison loop when comparing array against null

https://github.com/D-Programming-Language/dmd/commit/6f5a3cde5778e263d53d49f4546b668652ca89ae
Merge pull request #4574 from yebblies/issue14436

Issue 14436 - Optimizer fails to remove comparison loop when comparing array against null

[dlangBugzillaToGithub]

schuetzm commented on 2015-04-15T13:20:20Z

Comparing with an empty array or string literal still generates suboptimal code:

bool isEmpty(string s) {
        return s == "";
}

# dmd -c -O -inline -release xx.d

0000000000000000 <_D2xx7isEmptyFAyaZb>:
   0:	55                   	push   %rbp
   1:	48 8b ec             	mov    %rsp,%rbp
   4:	48 83 ec 10          	sub    $0x10,%rsp
   8:	48 89 7d f0          	mov    %rdi,-0x10(%rbp)
   c:	48 89 75 f8          	mov    %rsi,-0x8(%rbp)
  10:	48 3b 3d 00 00 00 00 	cmp    0x0(%rip),%rdi        # 17 <_D2xx7isEmptyFAyaZb+0x17>
  17:	75 15                	jne    2e <_D2xx7isEmptyFAyaZb+0x2e>
  19:	48 8b 75 f8          	mov    -0x8(%rbp),%rsi
  1d:	48 8b 3d 00 00 00 00 	mov    0x0(%rip),%rdi        # 24 <_D2xx7isEmptyFAyaZb+0x24>
  24:	48 8b 4d f0          	mov    -0x10(%rbp),%rcx
  28:	33 c0                	xor    %eax,%eax
  2a:	f3 a6                	repz cmpsb %es:(%rdi),%ds:(%rsi)
  2c:	74 04                	je     32 <_D2xx7isEmptyFAyaZb+0x32>
  2e:	31 c0                	xor    %eax,%eax
  30:	eb 05                	jmp    37 <_D2xx7isEmptyFAyaZb+0x37>
  32:	b8 01 00 00 00       	mov    $0x1,%eax
  37:	48 8b e5             	mov    %rbp,%rsp
  3a:	5d                   	pop    %rbp
  3b:	c3                   	retq   

Reopening, as it is a closely related enhancement (though strictly speaking not identical).

[dlangBugzillaToGithub]

yebblies commented on 2015-04-15T14:16:15Z

By empty array do you mean empty array literal?

eg assert(x != []);

[dlangBugzillaToGithub]

schuetzm commented on 2015-04-16T07:36:32Z

(In reply to yebblies from comment #4)
> By empty array do you mean empty array literal?
> 
> eg assert(x != []);

Yes, it should read "empty array [literal] or string literal".

In fact, the code generated for `int[]` instead of `string` looks even worse; there is a "call" instruction in it:

0000000000000000 <_D2xx7isEmptyFAiZb>:
   0:	55                   	push   %rbp
   1:	48 8b ec             	mov    %rsp,%rbp
   4:	48 83 ec 30          	sub    $0x30,%rsp
   8:	48 89 7d f0          	mov    %rdi,-0x10(%rbp)
   c:	48 89 75 f8          	mov    %rsi,-0x8(%rbp)
  10:	48 31 f6             	xor    %rsi,%rsi
  13:	48 bf 00 00 00 00 00 	movabs $0x0,%rdi
  1a:	00 00 00 
  1d:	e8 00 00 00 00       	callq  22 <_D2xx7isEmptyFAiZb+0x22>
  22:	48 89 c1             	mov    %rax,%rcx
  25:	48 31 c0             	xor    %rax,%rax
  28:	48 89 45 e0          	mov    %rax,-0x20(%rbp)
  2c:	48 89 4d e8          	mov    %rcx,-0x18(%rbp)
  30:	48 89 45 d0          	mov    %rax,-0x30(%rbp)
  34:	48 89 4d d8          	mov    %rcx,-0x28(%rbp)
  38:	48 8b 55 f0          	mov    -0x10(%rbp),%rdx
  3c:	48 3b 55 e0          	cmp    -0x20(%rbp),%rdx
  40:	75 1a                	jne    5c <_D2xx7isEmptyFAiZb+0x5c>
  42:	48 85 d2             	test   %rdx,%rdx
  45:	74 19                	je     60 <_D2xx7isEmptyFAiZb+0x60>
  47:	48 8b 75 f8          	mov    -0x8(%rbp),%rsi
  4b:	48 8b 7d d8          	mov    -0x28(%rbp),%rdi
  4f:	48 89 d1             	mov    %rdx,%rcx
  52:	48 c1 e1 02          	shl    $0x2,%rcx
  56:	33 c0                	xor    %eax,%eax
  58:	f3 a6                	repz cmpsb %es:(%rdi),%ds:(%rsi)
  5a:	74 04                	je     60 <_D2xx7isEmptyFAiZb+0x60>
  5c:	31 c0                	xor    %eax,%eax
  5e:	eb 05                	jmp    65 <_D2xx7isEmptyFAiZb+0x65>
  60:	b8 01 00 00 00       	mov    $0x1,%eax
  65:	48 8b e5             	mov    %rbp,%rsp
  68:	5d                   	pop    %rbp
  69:	c3                   	retq

[dlangBugzillaToGithub]

yebblies commented on 2015-04-16T14:48:48Z

Ok, I'll have a look.  This can probably be special-cased, although there is a long-standing problem that the optimizer doesn't look at constant data correctly.

[dlangBugzillaToGithub]

github-bugzilla commented on 2015-06-17T21:04:40Z

Commits pushed to stable at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/7660158f16b9c988d8ff16592ee442051ea95ffa
Fix Issue 14436 - Optimizer fails to remove comparison loop when comparing array against null

https://github.com/D-Programming-Language/dmd/commit/6f5a3cde5778e263d53d49f4546b668652ca89ae
Merge pull request #4574 from yebblies/issue14436

[dlangBugzillaToGithub]

schuetzm commented on 2015-06-19T17:51:12Z

Erroneously closed by the bot, reopening. Latest DMD still produces the assembly in comment #3.