digitalmars.D - #dbugfix Issue 16189
- Kirr (21/21) Feb 07 2018 https://issues.dlang.org/show_bug.cgi?id=16189
- Michael (8/31) Feb 07 2018 Yeah that's... really bad. I suppose the saving grace is that
- Bastiaan Veelo (5/7) Feb 07 2018 I played with it here https://run.dlang.io/is/15sr6c and every
- Mike Parker (2/4) Feb 07 2018 Noted!
https://issues.dlang.org/show_bug.cgi?id=16189 This is a P1 critical DMD bug, silently generating wrong code. Reported one and a half year ago, but exists in DMD compilers going back to DMD 2.050 (possibly more, but this is as far back as I tried). The repro is essentially C, it involves nothing but array, struct and loop. No templates, CTFE, no phobos, no any fancy features. I can fully understand that may be this bug is hard to fix, that there's not enough manpower, that there are other more pressing issues, that no one can be expected to work on something they are not interested in, etc. And may I'm the only one affected? (it's a mystery to me). But this bug is a phychological obstacle to my use of D (or rather DMD, because LDC seems OK). May be it's just my expectation that is too high. I'm not ready to give up arrays, structs or loops. I experimentally worked around this once I found it, by re-wording the loop. But I've no idea how to make sure this bug won't hit me somewhere else. I guess I can live without optimizer (or without DMD). In any case, I think that miscompiling a simplest C-like loop does not send the right message to a newcomer. I'll appreciate any help with this bug.
Feb 07 2018
On Wednesday, 7 February 2018 at 08:51:01 UTC, Kirr wrote:https://issues.dlang.org/show_bug.cgi?id=16189 This is a P1 critical DMD bug, silently generating wrong code. Reported one and a half year ago, but exists in DMD compilers going back to DMD 2.050 (possibly more, but this is as far back as I tried). The repro is essentially C, it involves nothing but array, struct and loop. No templates, CTFE, no phobos, no any fancy features. I can fully understand that may be this bug is hard to fix, that there's not enough manpower, that there are other more pressing issues, that no one can be expected to work on something they are not interested in, etc. And may I'm the only one affected? (it's a mystery to me). But this bug is a phychological obstacle to my use of D (or rather DMD, because LDC seems OK). May be it's just my expectation that is too high. I'm not ready to give up arrays, structs or loops. I experimentally worked around this once I found it, by re-wording the loop. But I've no idea how to make sure this bug won't hit me somewhere else. I guess I can live without optimizer (or without DMD). In any case, I think that miscompiling a simplest C-like loop does not send the right message to a newcomer. I'll appreciate any help with this bug.Yeah that's... really bad. I suppose the saving grace is that it's only a serious issue with the use of the optimiser, which I don't tend to use on DMD, but this should really be prioritised. I guess the fact that it's not a regression might make fixing it harder, but it doesn't really matter. Does it work with slightly varied examples like where a = -1, and is incremented etc.?
Feb 07 2018
On Wednesday, 7 February 2018 at 11:37:19 UTC, Michael wrote:Does it work with slightly varied examples like where a = -1, and is incremented etc.?I played with it here https://run.dlang.io/is/15sr6c and every variation I tried worked (correctly). So the example seems to be pretty minimal. Maybe someone understanding assembly is able to see what is going on after pressing that nice [ASM] button?
Feb 07 2018
On Wednesday, 7 February 2018 at 13:50:06 UTC, Bastiaan Veelo wrote:On Wednesday, 7 February 2018 at 11:37:19 UTC, Michael wrote:You don't need to understand assembler to grasp what's going on. A printf does a good job too: printf("%d\n", a); // 1908874352 It's even more fun, reduce the size of the static array to 8: ubyte[8][1] data; // works However, with 7 it fails again: ubyte[7][1] data; If you look at the assembly you will see that the compiler didn't even include the assert calls for even static arrays: https://run.dlang.io/is/Qkt1nA (-O) https://run.dlang.io/is/7UfmXJ (-O [8]) https://run.dlang.io/is/QrtNeI (normal) Here's an annotated excerpt from the normal build: --- dec qword ptr -010h[RBP] xor EDX,EDX ; a-- mov -8[RBP],DL test DL,DL ; if (b) je L5C jmp short L18 ; goto loop L5C: cmp qword ptr -010h[RBP],0FFFFFFFFFFFFFFFFh je L78 ; assert(a == -1) mov DL,0Ah lea RSI,_TMP0 PC32[RIP] lea RDI,_TMP0 PC32[RIP] call __assert PLT32 L78: xor EAX,EAX --- The same looks slightly different with -O. Here with printf and a different value to be compared with a because it's easier to read: --- main: push RBP mov RBP,RSP sub RSP,010h lea RAX,-010h[RBP] xor ECX,ECX mov [RAX],RCX mov 8[RAX],CL lea RSI,-010h[RBP] lea RDI,-010h[RBP] movsd movsb mov RSI,01C71C71C71C71C70h ; 2 function argument (value of a) lea RDI,FLAT:.rodata[00h][RIP] ; "%d" (1st function argument) xor EAX,EAX ; set eax to 0 call printf PLT32 ; printf("%d", a) mov EDX,0Ch lea RSI,_TMP0 PC32[RIP] ; load function arguments (in reverse order) lea RDI,_TMP0 PC32[RIP] call __assert PLT32 ; values load, let's call assert add [RAX],AL .text.main ends end ---- So the optimizer seems to be confused and wrongly precomputes a. Note that if you change something, a will be correctly statically set in the printf too: --- mov RSI,0FFFFFFFFh ; look ma - a is now -1 lea RDI,FLAT:.rodata[00h][RIP] xor EAX,EAX ; set eax to 0 call printf PLT32 ---Does it work with slightly varied examples like where a = -1, and is incremented etc.?I played with it here https://run.dlang.io/is/15sr6c and every variation I tried worked (correctly). So the example seems to be pretty minimal. Maybe someone understanding assembly is able to see what is going on after pressing that nice [ASM] button?
Feb 07 2018
Bastiaan Veelo wrote:On Wednesday, 7 February 2018 at 11:37:19 UTC, Michael wrote:there is a mix of loop strength reduction and data flow analysis. as inner loop contains array access and bounds checking, optimizer decided to turn `a` into actual index (i.e. multiply it by 16), and use subtraction in loop body. so far so good, optimizer did a good job. but then we have `assert(a == -1);` after the loop. and that is where things goes off the road: optimizer knows that it has `a` in register, and it knows that `a` was pre-multiplied, so it decided to divide `a` to 16 to get the real value. but... it does this with `shr` instruction instead of `sar`! most of the time it works as expected, but in our case... oops, we lost a sign. the fix should be fairly easy, but sorry, i can't get any sense from dmd backend. i see what it wrong due to my expirience with similar things, but that's all. here Walter (or some other backand guru) should step in.Does it work with slightly varied examples like where a = -1, and is incremented etc.?I played with it here https://run.dlang.io/is/15sr6c and every variation I tried worked (correctly). So the example seems to be pretty minimal. Maybe someone understanding assembly is able to see what is going on after pressing that nice [ASM] button?
Feb 07 2018
On Wednesday, 7 February 2018 at 18:46:50 UTC, ketmar wrote:...Great brainstorming, guys! Hopefuly the gained understanding will lead to eventual fix.
Feb 07 2018
On Wednesday, 7 February 2018 at 08:51:01 UTC, Kirr wrote:https://issues.dlang.org/show_bug.cgi?id=16189 I'll appreciate any help with this bug.Noted!
Feb 07 2018