fixes to std.math such that all unit tests pass on LDC 64bit windows.

[KevinBrogan]

fixes to std.math such that all unit tests pass on LDC 64bit windows.

[dnadlinger]

Thanks for your work!

Regarding the floating point constants, I'd really rather fix this once and for all by using a strtold that properly supports them. DMD has an implementation that we might be able to use (haven't checked the license). Apart from the fact that this also fixes the issue in user code, we generally try to minimize differences to upstream Phobos code. There is little chance that the change to use decimal constants would be accepted into the D-Programming-Language repo.

Concerning the 64 bit real issues, surely some of the people working on ARM support (e.g. @jpf91 for GDC) must have run into this too? Maybe you can reuse/share some work.

[KevinBrogan]

Any chance we can try with the floating point constants? lol. Seriously though, they are such a pain to work with.

If I am interpreting the 64 bit real issue correctly, the problem isn't one of processor support, but of calling conventions. Variables larger than 64 bits have to be stored in memory and can't be stored in registers, yet LDC stores them on the stack and pushes them into registers. I have no idea how to touch this.

Mind you, a week ago I didn't know about 64 bit calling conventions and a whole lot of other stuff... so who knows what I'll be able to do in the future.

[dnadlinger]

No worries, keep on experimenting. At this rate, you'll be a world-class compiler expert in no time. ;)

What I meant by "64 bit real issues" are the precision problems in the tests. I was just suggesting that maybe @jpf91, @ibuclaw or @smolt might already have some changes to make the tests work (maybe even in upstream Git master). At least, they might also be interested in the topic.

[KevinBrogan]

Is there a version that is for both win64 and LDC?

That was something else that I thought about.. shouldn't all of the version = whatever lines be all in one place?

[dnadlinger]

Is there a version that is for both win64 and LDC?

No, you need to manually nest them or define a new version locally (e.g. BrokenHexConstants, Real64BitWorkaround or so). This is by (Walter's) design.

That was something else that I thought about.. shouldn't all of the version = whatever lines be all in one place?

How do you mean in one place? version = <…> declarations are local to the current module.

[KevinBrogan]

I didn't realize that they had private scope. I just checked the predefined versions. I think I assumed that some of them must not have been predefined and were therefore imported.

[smolt]

I am interesting in following the topic. I too am working on 64-bit reals, specifically arm.

I think one common problem area for us in std.math is exp(). In the unit test you versioned out there is a test set for 80-bit real. I started on a test set for 64-bit reals but have not filled in the cases for under and overflow. I find a 1 bit difference in exp() results sometimes.

All other unittests work as expected except I get a funny answer from acosh() when optimization is enabled and subnormal behavior is different for some functions on iOS.

If you diff ios branch with release-0.15.0 branch, you can see where I made
changes.

https://github.com/smolt/phobos/blob/ios/std/math.d

[KevinBrogan]

A 1 bit difference is no biggy. Floating point math is not deterministic between different compilers or platforms. Any comparisons should be made with the std.math.approxEqual function instead of ==.

I'll take at your tests in the morning. Is there a standard somewhere that tells what tests should be made or are you just winging it?

[smolt]

No standard, just winging it with the 80-bit real exp() tests cases as starting point. Still need to calc some cases that are commented out.

I agree that 1 bit diff is usually a don't care, but coming from a place that built ASIC DSP, I usually want to understand the reason.

[JohanEngelen]

Hi Kevin,
Perhaps you can split these changes into a few smaller ones? That way a few of your changes can be merged-in. Also, some of the changes should go upstream, I think.

• The changes to sqrt and atan2 are clear improvements. For atan2, I wonder if it'd be better to use LLVM asm expressions (http://wiki.dlang.org/LDC_inline_assembly_expressions), for better codegen. I'm not sure, but I believe LLVM can reason better about such assembly code.
• The unittest for sqrt(-1) looks like it should go upstream!
• LDC+MSVC can now parse floating point constants: ldc-developers/ldc@2fdd4fe
• Instead of disabling some of the 80-bit real tests, we should add 64-bit tests (should go upstream). @smolt : are you going to submit your 64-bit tests upstream?

[smolt]

@JohanEngelen yes, after I completed the rest of the test set. If someone else gets to it first, that would be great as I have been involved in some other fun projects :-)

[redstar]

@KevinBrogan Thanks for the work! Like @JohanEngelen I think it is better to break this PR in parts.

@JohanEngelen Yes, using the LLVM assembly results in better code. E.g. DMD style assembly is not inlined.

[kinke]

I've come up with dlang#3285 to address the std.math.exp() issues. This is almost enough to pass the std.math unittests in debug on Win64, a one-liner is additionally needed for LDC to disable the IeeeFlags checks. With this PR we'll also be able to use the llvm.exp intrinsic without failing unittests (tested on Linux x64 too).

@smolt Would you please give it a shot on ARM?

[smolt]

@kinke - I only have iOS flavored ARM setup now and it has some unique properties. I don't think I can provide any additional input at this time besides some comments and that the real-64 test cases for exp() look good. I patched your dlang pull dlang#3285 into my os-merge-2.067 branch, but I still have IPhoneOS differences. Subnormal cases always result in zero and IEEE flags never get set for any cases.

On iOS, subnormals generally are not supported. ARM FPU Flush-To-Zero mode is enabled by default (subnormal math results become zero) by iOS runtime. When I explicitly disable Flush-To-Zero mode, libm math routines such as ldexp still flush subnormals . std.math.exp() uses libm ldexp() making all subnormal test cases end up with result of 0.0. Also, IEEE flags are not set by ldexp, nor by libm exp(), so none of the expected overflow/underflow cases ever pass. IEEE flags are tricky. Any math subroutine may clear them. And I still have ldc-developers/ldc#888 to think about.

[redstar]

I cut the sqrt() fix and the unit test and committed it. This is really a bug which affects all platforms.
I also created an upstream pull request for the unit test: dlang#3293
Thanks for spotting this!

[smolt]

Thanks! - All my std.math unittests pass for iOS now. The sqrt() test for negative args did wonders for the acosh() unittest in -O builds.

Out of curiosity, I had to study why acosh() worked in iOS debug builds. In debug builds, the optimizer doesn't get a chance to lower llvm_sqrt() intrinsic llvm.sqrt.f64 so ARM vsqrt.f64 instruction is generated, correctly producing NaN for negative args. With optimizer turned on, llvm.sqrt.f64 is lowered to LLVM undef and that seems to be treated like an uninitialized variable. Optimized assembly code for llvm_sqrt() uses whatever value is already in a floating point register leading to funny (undefined) results.

I see David documented this behavior in druntime ldc/intrinsics.di back in 2012.

[kinke]

Yep, that fixes the std.math-release issue(s) on Win64 too.

@smolt Wrt. the subnormal tests: parsing subnormal literals as 0 if the target doesn't support them would probably be the best solution and fix these feqrel() asserts, right? Wrt. the IEEE flags: the LLVM exp.f64() intrinsic doesn't set the flags either (at least on Win64), so I was planning on simply skipping the flags tests in LDC altogether.

[smolt]

@kinke - To handle subnormal cases on iOS, I pass all expected results from table through this function before comparing to exp() result:

    real expect(real x)
    {
        version(IPhoneOS)
            return isSubnormal(x) ? 0.0 : x;
        else
            return x;
    }

Maybe a specific version to opt-in or opt-out of flag IEEE testing for exp()? Is does work on some platforms. I wish for a way to know when IEEE flags are valid for checking, but seems like all corner cases.

[smolt]

@redstar - There is another instance of llvm_sqrt in druntime core.math that may need the nan recheck, depending on its purpose. Not sure if that module is supposed to be bare intrinsics.

https://github.com/ldc-developers/druntime/blob/ldc/src/core/math.d#L111

I noticed that checking for and returning nan before calling llvm_sqrt() prevents IEEE invalid flag from being set. Nothing to do about that, just a note because IEEE float flags have so many corner cases (e.g. ctfe or lowering removes float operation). So far only reliable way to get IEEE flags seems to be calling libm functions (core.stdc.math) or do inline assembly.

[redstar]

I am closing this PR because all Tests in Win64 are green now.