Bug 15619 – [REG 2.066] Floating-point x86_64 codegen regression, when involving array ops

To reproduce, build with: dmd -m64 -O testcase.d --------------- testcase.d -------------- import std.stdio; import std.math; double expDecayFactor(double time, double samplerate) pure nothrow @nogc { return 0.00045341191; } void processAudio(float* outputs, int frames) { float[] _mono; float[] _distancedL, _distancedR; _mono = new float[frames]; _distancedL = new float[frames]; _distancedR = new float[frames]; ExpSmoother smoother; smoother.initialize(86.0f, 0.05f, 0.05f, 1); _distancedL[] = 0; _distancedR[] = 0; float level = smoother.nextSample(); _mono[0..frames] = (_distancedL[0..frames] + _distancedR[0..frames]); for (int i = 0; i < frames; ++i) { outputs[i] = level; } } struct ExpSmoother { public: void initialize(float samplerate, float timeAttack, float timeRelease, float initialValue) { assert(isFinite(initialValue)); _current = cast(float)(1); _expFactorAttack = cast(float)(expDecayFactor(timeAttack, 44100.0f)); _expFactorRelease = _expFactorAttack; assert(isFinite(_expFactorAttack)); assert(isFinite(_expFactorRelease)); } float nextSample() { float target = 1; float diff = target - _current; if (diff != 0) { writeln("never passing here"); double expFactor = (diff > 0) ? _expFactorAttack : _expFactorAttack; double temp = _current + diff * expFactor; float newCurrent = cast(float)(temp); _current = newCurrent; } return _current; } private: float _current; float _expFactorAttack; float _expFactorRelease; } void main() { float[] buf = new float[16]; // must be >= 16 processAudio(buf.ptr, 16); // must be >= 16 writeln(buf); } -------------------------------------------- What is expected: Output array should be filled with ones. [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1] What we get: Output array is filled with zeros. [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] Ways to work-around the bug: - building for 32-bit - removing -O switch - removing the asserts - replace 16 by 15 as thenumber of elements - add a float unused field to ExpSmoother I've tested against several DMD versions: - 2.067.1 => works - 2.068.1 => doesn't work - 2.069.0 => doesn't work - 2.070.0 => doesn't work

Reduced to: -------------------------------- import std.stdio; void fillWithOnes(float* outputs, int frames) { float[] A = new float[frames]; float[] B = new float[frames]; A[] = 0; B[] = 0; float one = returnOne(); A[0..frames] += B[0..frames]; for (int i = 0; i < frames; ++i) { outputs[i] = one; } } float returnOne() { return 1; } void main() { float[] buf = new float[16]; fillWithOnes(buf.ptr, 16); writeln(buf); } --------------------------------

Both original and reduced code generate wrong result since 2.066.0 (2.065 is ok).

The array operation uses xmm6/xmm7 internally without saving them, but they are callee saved on win64. Probably introduced by this: https://github.com/D-Programming-Language/druntime/pull/829

I'll take this. https://trello.com/c/TNCQ6xBk/171-issue-15619-reg-2-066-floating-point-x86-64-codegen-regression-when-involving-array-ops

As this is already broken for a while (and way longer for other array ops, e.g. double), I'll delay this fix for a proper replacement w/ templated array ops. https://github.com/D-Programming-Language/druntime/pull/471#issuecomment-16075234 WIP: https://github.com/MartinNowak/dmd/tree/arrayOps https://github.com/MartinNowak/druntime/tree/arrayOps

Doesn't seem like this should be too hard to fix - just save xmm6-7 for Win64 - why not just do it?

FWIW I don't use array ops anymore, to the exception of: slice[] = value; sliveA[] = sliceB[];

(In reply to Walter Bright from comment #6) > Doesn't seem like this should be too hard to fix - just save xmm6-7 for > Win64 - why not just do it? Because all the other functions ignore it as well and maintaining that mess is just wasted time.

Commit pushed to master at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/184435f243b830cf464047deb1b636a8d5b4ed4a implement templated array ops - use RPN to encode operand precedence - fixes Issue 15619, and 16680