Bug 12833 – switch statement does not work properly when -inline used

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Windows
Creation time
2014-06-01T07:48:00Z
Last change time
2014-08-22T08:04:39Z
Keywords
pull, wrong-code
Assigned to
nobody
Creator
jiki

Comments

Comment #0 by jiki — 2014-06-01T07:48:19Z
The following code does not work properly with -inline -- the program finishes silently. I tested on DMD2.065, git-head, and DMD1.076. CODE: import std.stdio; ushort HIWORD(long x) { return cast(ushort)((x >> 16) & 0xFFFF); } void test(int a) { switch (HIWORD(a)) { case 1: writefln("it works!"); // this makes no output if -inline used break; default: } } void main() { test(0x1_0000); } COMMAND: dmd -inline -m32 -run test.d ENVIRONMENT: Windows 7 64-bit
Comment #1 by k.hara.pg — 2014-06-03T08:27:49Z
This is dmd backend isuse rather than -inline switch. The test function is inlined to: long x = cast(long)a; switch (cast(int)(cast(ushort)(x >> 16 & 65535L))) { case 1: { writefln("it works!"); break; } default: { } } And the condition of switch statement is always 0. Reduced test case (can reproduce the issue without -inline): extern(C) int printf(const char*, ...); void main() { int a = 0x1_0000; //00402016: b800000100 mov eax, 0x10000 //0040201b: 8945ec mov [ebp-0x14], eax long x = cast(long)a; //0040201e: 89c2 mov edx, eax //00402020: c1fa1f sar edx, 0x1f //00402023: 8945f4 mov [ebp-0xc], eax //00402026: 8955f8 mov [ebp-0x8], edx int num = cast(int)(cast(ushort)(x >> 16 & 65535L)); // problem! //00402029: 0facd010 shrd eax, edx, 0x10 //0040202d: c1fa10 sar edx, 0x10 //00402030: 25ffff0000 and eax, 0xffff //00402035: 31d2 xor edx, edx // <-- //00402037: 8955fc mov [ebp-0x4], edx // <-- if (num == 1) //0040203a: 83fa01 cmp edx, 0x1 //0040203d: 7510 jnz 0x40204f D main test.d:15 { printf("it works!\x0a"); } else assert(0); // line 15 }
Comment #2 by yebblies — 2014-06-03T11:54:13Z
So, the problem is: 1. the code is emitted to bitwise and a long with 0xFFFF (and EAX with 0xFFFF, and EDX with 0x0) 2. the compiler checks and decides to do nothing for the long -> ushort conversion because the result is already in AX where it should be. 3. the zext code has a really dodgy check (cdshtlng cod4.c:2905) that promotes (r16 & mask16) into (r32 & mask32), but doesn't bother checking that it's actually anding the correct register - in this case it sees the "and EDX, 0" and does nothing. It also sets the return register to whatever that instruction was using and as a result later code-gen stupidly uses edx. Making this change appears to fix it by disallowing the 'optimization' if the register is wrong: @@ -2900,11 +2900,11 @@ code *cdshtlng(elem *e,regm_t *pretregs) c1 = codelem(e1,&retregs,FALSE); c2 = getregs(retregs); if (op == OPu16_32 && c1) { code *cx = code_last(c1); - if (cx->Iop == 0x81 && (cx->Irm & modregrm(3,7,0)) == modregrm(3,4,0)) + if (cx->Iop == 0x81 && (cx->Irm & modregrm(3,7,0)) == modregrm(3,4,0) && (1 << (cx->Irm & modregrm(0,0,7))) == retregs) { // Convert AND of a word to AND of a dword, zeroing upper word retregs = mask[cx->Irm & 7]; if (cx->Irex & REX_B) retregs = mask[8 | (cx->Irm & 7)]; Hopefully Walter can comment on the correctness of this patch.
Comment #3 by yebblies — 2014-07-16T08:43:31Z
Comment #4 by github-bugzilla — 2014-07-17T22:46:55Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/82da99d83bc62d043b444885f38c90d9a9cb4b99 Fix Issue 12833 - switch statement does not work properly when -inline used https://github.com/D-Programming-Language/dmd/commit/e701790c2830c4cad03edef6640e111caee13d4e Merge pull request #3776 from yebblies/issue12833 Issue 12833 - switch statement does not work properly when -inline used
Comment #5 by github-bugzilla — 2014-07-20T20:59:51Z
Commit pushed to 2.066 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/72af20199bc1caaab8a4457a5960bcbb677457c6 Merge pull request #3776 from yebblies/issue12833 Issue 12833 - switch statement does not work properly when -inline used
Comment #6 by github-bugzilla — 2014-08-22T08:04:39Z