Issue 5323 - std.math: struct FloatingPointControl, duplicate code and assumes X86
Summary: std.math: struct FloatingPointControl, duplicate code and assumes X86
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: Other All
: P3 normal
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-05 08:01 UTC by Iain Buclaw
Modified: 2024-12-01 16:13 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Iain Buclaw 2010-12-05 08:01:19 UTC
Excerpt from struct FloatingPointControl:

/** IEEE hardware exceptions.
 *  By default, all exceptions are masked (disabled).
 */
enum : uint
{
    inexactException   = 0x20,
    underflowException = 0x10,
    overflowException  = 0x08,
    divByZeroException = 0x04,
    invalidException   = 0x01,
    /// Severe = The overflow, division by zero, and invalid exceptions.
    severeExceptions   = overflowException | divByZeroException
                         | invalidException,
    allExceptions      = severeExceptions | underflowException
                         | inexactException,
};


A little further up the file in struct IeeeFlags:

// The x87 FPU status register is 16 bits.
// The Pentium SSE2 status register is 32 bits.
uint flags;
version (X86_Any) {
    // Applies to both x87 status word (16 bits) and SSE2 status word(32 bits).
    enum : int {
        INEXACT_MASK   = 0x20,
        UNDERFLOW_MASK = 0x10,
        OVERFLOW_MASK  = 0x08,
        DIVBYZERO_MASK = 0x04,
        INVALID_MASK   = 0x01
    }
    // Don't bother about denormals, they are not supported on most CPUs.
    //  DENORMAL_MASK = 0x02;
} else version (PPC) {



Both implement the same thing, but have (rather confusingly) different names and one only implements for X86 whereas the other (at least tries) to implement PPC and SPARC too.

IMO, as both structs take their values from <fpu_control.h> for GlibC (<machine/ieeeflags.h> for FreeBSD?? <float.h> for Windows???), these enums (and possibly the Get/Set/Clear CW functions too) should be consolidated into one place elsewhere. For example, std.internal.math.fpbits, or possibly DRuntime...

Regards
Comment 1 Iain Buclaw 2010-12-05 08:30:25 UTC
Hmm... also found in <fenv.h>, so perhaps should just be importing core.stdc.fenv (merging any bits added by std.math).
Comment 2 Infiltrator 2015-11-01 02:43:21 UTC
Is this still an issue?  I'd have a look at the source, but I'm afraid that I wouldn't be able to tell what's X86 or SPARC, etc.
Comment 3 Iain Buclaw 2015-11-01 07:59:10 UTC
(In reply to Infiltrator from comment #2)
> Is this still an issue?  I'd have a look at the source, but I'm afraid that
> I wouldn't be able to tell what's X86 or SPARC, etc.

Well, X86 code is guarded by a version(X86) block, SPARC code by a version(SPARC) block.  :-)
Comment 4 Iain Buclaw 2015-11-01 08:13:25 UTC
Both struct FloatingPointControl and Ieeeflags still have their own private enums.  Since these values should be mirrored somewhere in druntime stdc (it looks like the me of five years ago couldn't work out where when searching through the libc headers), then yes this is still a valid refactoring.
Comment 5 Guillaume Chatelet 2017-03-03 12:05:22 UTC
It seems to me that setting rounding mode and testing for inexact does not work correctly on x86_64.

Correct code using the C bindings:
void main() {
    import core.stdc.fenv;
    fesetround(FE_UPWARD);
    float x = 1.0f;
    x += float.min_normal;
    writefln("%.32g, inexact: %s", x, fetestexcept(FE_INEXACT) > 0);
}

> Output: 1.00000011920928955078125, inexact: true

The same code using FloatingPointControl and IeeeFlags does not work at all (neither set round nor test except)
void main() {
    resetIeeeFlags();
    FloatingPointControl fpctrl;
    fpctrl.rounding = FloatingPointControl.roundUp;
    float x = 1.0f;
    x += float.min_normal;
    writefln("%.32g, inexact: %s", x, ieeeFlags.inexact);
}

> Output: 1, inexact: false

Linked discussion: http://forum.dlang.org/post/qybweycrifqgtcssepgx@forum.dlang.org
Comment 6 Iain Buclaw 2018-02-10 14:12:02 UTC
Has been improved somewhat by:

https://github.com/dlang/phobos/pull/4272
https://github.com/dlang/phobos/pull/5769
Comment 7 dlangBugzillaToGithub 2024-12-01 16:13:42 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/phobos/issues/9582

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB