Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using deprecated floating-point environment functions on FreeBSD #68708

Closed
AndrewTurner mannequin opened this issue Jun 27, 2015 · 16 comments
Closed

Stop using deprecated floating-point environment functions on FreeBSD #68708

AndrewTurner mannequin opened this issue Jun 27, 2015 · 16 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@AndrewTurner
Copy link
Mannequin

AndrewTurner mannequin commented Jun 27, 2015

BPO 24520
Nosy @mdickinson, @vstinner, @skrah, @koobs
Files
  • use_fenv_freebsd.diff: Patch to Programs/python.c to use the fenv functions
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2016-01-22.16:04:35.647>
    created_at = <Date 2015-06-27.15:08:22.984>
    labels = ['interpreter-core', 'type-bug']
    title = 'Stop using deprecated floating-point environment functions on FreeBSD'
    updated_at = <Date 2016-01-22.16:04:35.645>
    user = 'https://bugs.python.org/AndrewTurner'

    bugs.python.org fields:

    activity = <Date 2016-01-22.16:04:35.645>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-01-22.16:04:35.647>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2015-06-27.15:08:22.984>
    creator = 'Andrew Turner'
    dependencies = []
    files = ['39822']
    hgrepos = []
    issue_num = 24520
    keywords = ['patch', 'needs review']
    message_count = 16.0
    messages = ['245879', '251221', '251227', '251228', '251230', '251231', '251258', '257429', '258690', '258691', '258692', '258694', '258695', '258720', '258721', '258819']
    nosy_count = 8.0
    nosy_names = ['mark.dickinson', 'vstinner', 'Arfrever', 'skrah', 'python-dev', 'koobs', 'emaste', 'Andrew Turner']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue24520'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @AndrewTurner
    Copy link
    Mannequin Author

    AndrewTurner mannequin commented Jun 27, 2015

    The attached patch moves to use the fenv functions on FreeBSD to control the floating-point environment. This will be needed as I don't expect FreeBSD will have these functions on arm64.

    I would expect a similar change should be applied to any supported development branch.

    @AndrewTurner AndrewTurner mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jun 27, 2015
    @vstinner
    Copy link
    Member

    It looks like fpsetmask() was deprecated since many years, and that fedisableexcept() is available since FreeBSD 5.3. Since we try to support FreeBSD 9, it's fine to drop support of FreeBSD < 5.3.

    I'm concerned by the "CAVEATS" section below. Should we put "#pragma STDC FENV_ACCESS ON" in all .c file using a C double?

    @skrah: I saw this pragma in Modules/_decimal/libmpdec/mpdecimal.c. Any idea if applying this patch is fine?

    Maybe we can start by applying it to the default branch?

    If this patch is required to support arm64, I think that it's ok to apply it to 2.7, 3.4 and 3.5 since we still accept changes to fix platform compatibility issues.

    http://www.unix.com/man-page/freebsd/3/fpsetmask/
    ---
    DESCRIPTION
    The routines described herein are deprecated. New code should use the functionality provided by fenv(3).
    ---

    http://www.unix.com/man-page/freebsd/3/fenv/
    ---
    HISTORY
    The <fenv.h> header first appeared in FreeBSD 5.3. It supersedes the non-standard routines defined in <ieeefp.h> and documented in fpgetround(3).

    CAVEATS
    The FENV_ACCESS pragma can be enabled with
    #pragma STDC FENV_ACCESS ON
    ---

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 21, 2015

    In theory we should set FENV_ACCESS whenever the control word is
    changed (C99):

    "If part of a program tests floating-point status flags, sets
    floating-point control modes, or runs under non-default mode
    settings, but was translated with the state for the FENV_ACCESS
    pragma "off", the behavior is undefined."

    In practice gcc ignores the pragma, icc and cl.exe use it, not
    sure about clang.

    @vstinner
    Copy link
    Member

    FYI in Python/pytime.c, I put a lot of "volatile double" when tests started to fail on some platforms. It's inefficient, but it's easier than teaching how to disable optimizations on each different compiler :-p

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 21, 2015

    Ok, clang does not support it either:

    https://llvm.org/bugs/show_bug.cgi?id=10409

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Sep 21, 2015

    Regarding volatile: gcc/clang seem to honor *some* changes to
    the control word. At least in libmpdec both compilers have
    always left the settings 80bit-prec/ROUND_CHOP intact, even
    though it's not the default.

    Let's rewrite Python in asm to avoid these issues. :)

    @vstinner
    Copy link
    Member

    Sorry, I don't understand if we should apply or not the patch :-/

    @AndrewTurner
    Copy link
    Mannequin Author

    AndrewTurner mannequin commented Jan 3, 2016

    Can this be applied?

    @koobs
    Copy link

    koobs commented Jan 20, 2016

    This issue is becoming increasingly important as FreeBSD 11.0-RELEASE time nears.

    What remains to be done/identified/answers here to make progress?

    @vstinner
    Copy link
    Member

    Andrew Turner: "Can this be applied?"

    koobs: "What remains to be done/identified/answers here to make progress?"

    *I* don't understand if replacing fpsetmask() with fedisableexcept() is enough, or if we also need to starting putting "#pragma STDC FENV_ACCESS ON" in all .c file using a C double? It's not a rhetorical question, I don't understand the consequence of the attached patch.

    If two FreeBSD developers (Andrew Turner and koobs) are confident, maybe I should trust them and apply blindly the patch without trying to understand it :-D

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 20, 2016

    1. Feedback whether the compiler used on FreeBSD recognizes FENV_ACCESS.

    2. If not, an indication why the man page mentions it.

    @AndrewTurner
    Copy link
    Mannequin Author

    AndrewTurner mannequin commented Jan 20, 2016

    I think this patch is correct.

    Clang, as shipped by FreeBSD, doesn't support FENV_ACCESS. It raises the following warning:

    fenv_test.c:2:14: warning: pragma STDC FENV_ACCESS ON is not supported, ignoring pragma [-Wunknown-pragmas]
    #pragma STDC FENV_ACCESS ON
    ^
    1 warning generated.

    I expect the man page mentions it because it is mentioned in the standard. In the bugs section it does say the pragma is unimplemented.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 20, 2016

    Thanks, then the patch looks good to me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 20, 2016

    New changeset 76f35f35be50 by Victor Stinner in branch '3.5':
    Replace fpgetmask() with fedisableexcept()
    https://hg.python.org/cpython/rev/76f35f35be50

    New changeset 6134d9ecab60 by Victor Stinner in branch 'default':
    Merge 3.5 (issue bpo-24520)
    https://hg.python.org/cpython/rev/6134d9ecab60

    New changeset 394ae9efc5c2 by Victor Stinner in branch '2.7':
    Replace fpgetmask() with fedisableexcept()
    https://hg.python.org/cpython/rev/394ae9efc5c2

    @vstinner
    Copy link
    Member

    "I expect the man page mentions it because it is mentioned in the standard. In the bugs section it does say the pragma is unimplemented."

    Ah ah, funnny. Sorry, I was confused by the manual page.

    I pushed your patch to Python 2.7, 3.5 and default (3.6) branches.

    @koobs: sorry, 3.4 now only accept security fixes :-p

    @vstinner
    Copy link
    Member

    @koobs: sorry, 3.4 now only accept security fixes :-p

    I wrote a table giving the status of each Python branch to know which ones still accept bugfixes or not:
    https://docs.python.org/devguide/#status-of-python-branches

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants