Bug 22372 – Loop index incorrectly optimised out for -release -O

When compiling the code below with the -O -release flags, the index used by the for loop gets optimised out by the compiler. Thus, when throws2ndCall throws, the value of i in the catch body is 0 instead of 1. This behaviour seems to occur when the size of S is somewhat larger. When S only contains 1 or 2 ints, the behaviour is the one expected. The error appears from a size of 3 ints and above. static size_t n; void throws2ndCall(T)(ref T x) { if (n == 1) throw new Exception("n == 1"); ++n; } void foo(T)(scope T[] arr) { size_t i; try { for (i = 0; i < arr.length; i++) throws2ndCall(arr[i]); } catch (Exception o) { printf("Exception: i = %lu; n = %lu\n", i, n); // prints 0 and 1 assert(i == n); // this fails } } void main() { static struct S { int a1, a2, a3; } foo([S(), S()]); } DMD64 D Compiler v2.098.0: -release -O

Using run.dlang.io, I tried to find a regression, but all supported DMD versions (2.060 and newer) are printing the same incorrect output (Exception: i = 0; n = 1). My experiment can be found here: https://run.dlang.io/is/uYfTzl. This seems to be a backend issue, as both LDC and LDC-beta are working fine. However, I came across this bug when working on this PR: https://github.com/dlang/dmd/pull/13116. And when analysing the CI outputs, I noticed that all 3 of DMD, LDC and GDC are failing the same test, which I've also been able to reproduce myself: - LDC: https://cirrus-ci.com/task/6291197929979904?logs=test_druntime#L1449 - DMD: https://cirrus-ci.com/task/5728247976558592?logs=test_druntime#L1390 - GDC: https://cirrus-ci.com/task/4883823046426624?logs=test_druntime#L1411 The failure of this test is caused by the issue I mentioned above and the code with which I reproduced the bug is based on it.

(In reply to Teodor Dutu from comment #1) > However, I came across this bug when working on this PR: > https://github.com/dlang/dmd/pull/13116. And when analysing the CI outputs, > I noticed that all 3 of DMD, LDC and GDC are failing the same test, which > I've also been able to reproduce myself: > - LDC: https://cirrus-ci.com/task/6291197929979904?logs=test_druntime#L1449 > - DMD: https://cirrus-ci.com/task/5728247976558592?logs=test_druntime#L1390 > - GDC: https://cirrus-ci.com/task/4883823046426624?logs=test_druntime#L1411 > > The failure of this test is caused by the issue I mentioned above and the > code with which I reproduced the bug is based on it. There is no bug in GDC or LDC. $ gdc -O2 -frelease pr22372.d && ./a.out Exception: i = 1; n = 1 $ ldc2 -O -release pr22372.d && ./pr22732 Exception: i = 1; n = 1 $ dmd -O -release pr22372.d && ./pr22732 Exception: i = 0; n = 1

If you make size_t i static the behaviour is correct

(In reply to Imperatorn from comment #3) > If you make size_t i static the behaviour is correct Yes, this would fix the issue, but it would be a hack rather than a solution. The bug would still persist in DMD's backend.

Lol, of course. It was just an observation. Another observation: If you change type from size_t to int or uint the behaviour is correct

Hello, I've worked on the bug a bit and found the problem location in the backend, in gloop, which performs loop optims: `i` is optimized in the code that follows [1]. [1] https://github.com/dlang/dmd/blob/master/src/dmd/backend/gloop.d#L2966

by digging further it looks like the problem resides in dmd.backend.gloop reference counting system. OPInd would be ignored, so when a loop body only contains an IndexExp and that the IndexExp subscript is the IV (increment value ?), DMD thinks that the IV is not used.

@teodutu created dlang/druntime pull request #3585 "Use workaround until issue 22372 is fixed" mentioning this issue: - Use workaround until issue 22372 is fixed Signed-off-by: Teodor Dutu <[email protected]> https://github.com/dlang/druntime/pull/3585

dlang/druntime pull request #3585 "Use workaround until issue 22372 is fixed" was merged into master: - 06615791bcd20ed0edba68869babf76c22787caf by Teodor Dutu: Use workaround until issue 22372 is fixed Signed-off-by: Teodor Dutu <[email protected]> https://github.com/dlang/druntime/pull/3585

A simplified version: import core.stdc.stdio; struct S { int a1, a2, a3; } void throws2ndCall(ref S x) { __gshared bool b; if (b) throw new Exception("n == 1"); b = true; } void main() { S[] arr = [S(), S()]; size_t i; try { for (i = 0; i < 2; i++) throws2ndCall(arr[i]); } catch (Exception o) { printf("Exception: i = %lu\n", i); // prints 0 assert(i == 1); // this fails } }

@WalterBright created dlang/dmd pull request #13158 "fix Issue 22372 - Loop index incorrectly optimised out for -release -O" fixing this issue: - fix Issue 22372 - Loop index incorrectly optimised out for -release -O https://github.com/dlang/dmd/pull/13158

dlang/dmd pull request #13158 "fix Issue 22372 - Loop index incorrectly optimised out for -release -O" was merged into master: - 3e2a24503b338def775a3d65ca756de63a3efd3d by Walter Bright: fix Issue 22372 - Loop index incorrectly optimised out for -release -O https://github.com/dlang/dmd/pull/13158