`std.math.FloatingPointControl` fixes #1208

Merged
merged 3 commits into from Mar 23, 2013

Conversation

Projects
None yet
5 participants
Contributor

denis-sh commented Mar 14, 2013

std.math.FloatingPointControl is drammatically incorrect without this pull.

denis-sh added some commits Mar 14, 2013

@denis-sh denis-sh [whitespace] Fix spaces in `std.math.FloatingPointControl` example an…
…d unittest.
9bf70d5
@denis-sh denis-sh Fix `std.math.FloatingPointControl` two issues.
1. Do not call `setControlState` in destructor if not initialized (previously results in enabling all exceptions).
2. Call `initialize` on setting rounding mode (previously not called).
99e3440
@denis-sh denis-sh [docs] Improve `std.math.FloatingPointControl` example. 6fb40fb

donc was assigned Mar 14, 2013

Owner

andralex commented Mar 14, 2013

assigned @donc

@don-clugston-sociomantic don-clugston-sociomantic commented on the diff Mar 15, 2013

std/math.d
fpctrl.enableExceptions(FloatingPointControl.severeExceptions);
- double y = x*3.0; // will generate a hardware exception, if x is uninitialized.
- //
+ // This will generate a hardware exception, if x is a
+ // default-initialized floating point variable:
+ real x; // Add `= 0` or even `= real.nan` to not throw the exception.
@don-clugston-sociomantic

don-clugston-sociomantic Mar 15, 2013

Contributor

Yes, that it what it does, but it's a bug -- it's not supposed to generate an exception.
Actually whether it throws an exception or not is different depending on whether you are on an AMD or Intel processor.

@denis-sh

denis-sh Mar 15, 2013

Contributor

If I don't miss something it is a documented behavior. But also I still don't understand how exactly it behaves from you comment. Could you please clarify the documentation (e.g. defining platform depending behavior)?

@don-clugston-sociomantic

don-clugston-sociomantic Mar 15, 2013

Contributor

real x;
should not generate an exception. An exception should only be generated if x is actually used. But the compiler is incorrectly inserting a load instruction when it initializes the variable. This is a compiler bug, and it's my fault, I've never got around to fixing it. It makes the feature practically useless at the moment. The funny thing is that Intel processors do not generate exceptions when they load 80-bit signalling NaNs, but AMD processors do, so the bug doesn't always have an effect. (I don't think the difference between the processors is intentional, I think AMD just didn't read Intel's docs carefully enough). Both processors will generate an exception as soon as you do an operation on a signalling NaN.

@denis-sh

denis-sh Mar 15, 2013

Contributor

Being an Intel user I didn't know about such AMD behavior. The comment "This will generate..." was about the second line (as for me it was "obviously" a source of exception).

@denis-sh

denis-sh Mar 15, 2013

Contributor

Could you add your comment to FloatingPointControl's documentation? With addition of "Defining platform depending behavior" section and "Bugs" section? It will really help a lot for people to understand what really happens.

Contributor

don-clugston-sociomantic commented Mar 15, 2013

Looks OK apart from the issue I mentioned.

@klickverbot klickverbot added a commit that referenced this pull request Mar 23, 2013

@klickverbot klickverbot Merge pull request #1208 from denis-sh/std.math.FloatingPointControl-…
…fixes

`std.math.FloatingPointControl` fixes
73aa70f

@klickverbot klickverbot merged commit 73aa70f into dlang:master Mar 23, 2013

1 check was pending

default Pass: 8, In Progress: 1, Pending: 1
Details
Member

klickverbot commented Mar 23, 2013

@donc: Could you take care of adding any necessary bug reports/documentation notes please? As far as I can tell, the issue you mentioned is not really related to this pull request, and you are our resident IEEE 754 expert.

Contributor

don-clugston-sociomantic commented Mar 25, 2013

@klickverbot: The issue I mentioned is directly relevant. There are two lines of changes to the documentation in this pull request, which are wrong. Basically it's documenting a bug. I have created a bug report:
http://d.puremagic.com/issues/show_bug.cgi?id=9813

Contributor

denis-sh commented Mar 25, 2013

@don-clugston-sociomantic, thanks for the issue. What about documentation improvement?

Contributor

don-clugston-sociomantic commented Mar 26, 2013

The three lines I commented on, should simply be removed. AFAIK there is nothing in the spec which refers to implementation bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment