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

Enable short float repr() on Solaris/x86 #50042

Closed
mdickinson opened this issue Apr 19, 2009 · 22 comments
Closed

Enable short float repr() on Solaris/x86 #50042

mdickinson opened this issue Apr 19, 2009 · 22 comments
Assignees
Labels
easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@mdickinson
Copy link
Member

BPO 5792
Nosy @mdickinson, @skrah
Files
  • sun_short_float_repr.patch
  • 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 = 'https://github.com/mdickinson'
    closed_at = <Date 2009-11-15.13:49:49.287>
    created_at = <Date 2009-04-19.12:33:18.896>
    labels = ['interpreter-core', 'easy', 'type-feature']
    title = 'Enable short float repr() on Solaris/x86'
    updated_at = <Date 2009-11-15.13:49:49.286>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2009-11-15.13:49:49.286>
    actor = 'mark.dickinson'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2009-11-15.13:49:49.287>
    closer = 'mark.dickinson'
    components = ['Interpreter Core']
    creation = <Date 2009-04-19.12:33:18.896>
    creator = 'mark.dickinson'
    dependencies = []
    files = ['15299']
    hgrepos = []
    issue_num = 5792
    keywords = ['patch', 'easy']
    message_count = 22.0
    messages = ['86168', '86169', '95065', '95068', '95069', '95071', '95072', '95073', '95076', '95077', '95078', '95079', '95080', '95082', '95083', '95085', '95087', '95088', '95093', '95094', '95097', '95289']
    nosy_count = 2.0
    nosy_names = ['mark.dickinson', 'skrah']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue5792'
    versions = ['Python 3.1']

    @mdickinson
    Copy link
    Member Author

    The short float repr() that's in 3.1 (probably) doesn't work on
    Solaris/x86 when using Sun's compiler to build. It would be nice to
    fix this.

    It probably works on Solaris/x86/gcc, though confirmation of that would be
    appreciated.

    Marking as easy, because this should be an easy task for someone who has
    access to the appropriate setup. I can probably do all the
    autoconf/define stuff required, but I need help from someone who has
    access to Solaris/x86 and who can be available to do testing.

    @mdickinson mdickinson added interpreter-core (Objects, Python, Grammar, and Parser dirs) easy type-feature A feature request or enhancement labels Apr 19, 2009
    @mdickinson
    Copy link
    Member Author

    See the comments in the Include/pyport.h file for an explanation of
    what's required. (Search for HAVE_PY_SET_53BIT_PRECISION.)

    @mdickinson
    Copy link
    Member Author

    There are some related comments in issue bpo-7281.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    I can confirm that short float repr() is active and all float tests are
    passed on this combination:

    Ubuntu64bit -> KVM -> OpenSolaris32bit/Python3.2/gcc

    @mdickinson
    Copy link
    Member Author

    Stefan Krah mentions in the bpo-7281 discussion that suncc supports
    the C99 fenv functions. I'm not sure how to use these to set the x87
    precision, though. (Setting the rounding mode is straightforward.)

    @mdickinson
    Copy link
    Member Author

    I see two alternatives:

    (1) Use fesetenv. I don't know what the appropriate value to pass would
    be though, or even whether solaris lets you use fesetenv to control the
    x87 precision. It seems that its primary purpose is to set flags and traps.

    (2) Use inline assembly, assuming that suncc supports this. This seems
    simpler. It's just a matter of figuring out the syntax that suncc
    expects for asm, and making sure the code is properly tested.

    @mdickinson
    Copy link
    Member Author

    Looking at:

    http://docs.sun.com/app/docs/doc/816-5172/fesetenv-3m

    it seems that fesetenv isn't what we want here. It 'only installs the
    state of the floating-point status flags represented through its argument'.

    @mdickinson
    Copy link
    Member Author

    Stefan, is it possible that suncc already accepts the assembler syntax
    used in Python/pymath.h (py3k or trunk) for the functions
    _Py_get_387controlword and _Py_set_387controlword?

    @mdickinson mdickinson self-assigned this Nov 9, 2009
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    The inline asm compiles, but I don't know how good the GNU inline asm
    support is with suncc in general. I'm not a heavy user of suncc, I just
    use it for testing.

    That said, perhaps fesetprec works, too:

    http://docs.sun.com/app/docs/doc/816-5172/fesetprec-3m

    @mdickinson
    Copy link
    Member Author

    Excellent! From a bit of searching, it looks as though this assembler
    syntax works on icc as well, which is very good news.

    Thanks for finding fesetprec as well. It's a shame this isn't standard
    C. Oh well; maybe for C201X. I think I'd prefer to stick with the
    inline assembly, since it seems that there's very little to do to make
    this just work.

    Next problem: when compiling with suncc, how do I detect (in the
    configure script)

    (1) that I'm using suncc, and
    (2) whether the hardware is x86 or not (preferably excluding the case of
    x86-64).

    For gcc, configure.in is using:

    if test -n "$CC -dM -E - </dev/null | grep i386"

    to detect whether we're on x86. I guess it's too much to hope for that
    this works for suncc as well.

    @mdickinson
    Copy link
    Member Author

    fesetprec and fegetprec are at least semi-standard, it seems. They're
    recommended in the C99 rationale (see page 121 of

    http://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10.pdf

    ).

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    If gcc and suncc are present, ./configure chooses gcc and everything is
    fine.

    If only suncc is present, it's detected as cc. These tests should be
    possible:

    stefan@opensolaris:/svn/py3k$ cc -V
    cc: Sun C 5.9 SunOS_i386 Patch 124868-07 2008/10/07
    usage: cc [ options] files. Use 'cc -flags' for details
    stefan@opensolaris:
    /svn/py3k$ if cc -V 2>&1 | grep -q 'SunOS_i386';
    then echo yes; fi
    yes
    stefan@opensolaris:/svn/py3k$ uname -a
    SunOS opensolaris 5.11 snv_101b i86pc i386 i86pc Solaris
    stefan@opensolaris:
    /svn/py3k$ if uname -a | grep -q i386; then echo yes; fi
    yes

    @mdickinson
    Copy link
    Member Author

    Thanks. uname looks like the way to go, then.

    Is your copy of OpenSolaris running in 32-bit mode or 64-bit mode? Does
    the mode make a difference to the output of uname, or is uname -p always
    i386, regardless of the mode?

    I think the configure test for the inline assembly should go ahead on
    both x86 and x86-64: it seems likely that a 64-bit OS would be using
    SSE2 instructions for floating-point (which would make setting and
    getting the x87 control word unnecessary) instead of the x87 FPU, but I
    don't know that for sure.

    Actually, I guess I could just make that configure test unconditional.
    It'll fail on non-x86 hardware, but that's no big deal.

    @mdickinson
    Copy link
    Member Author

    Stefan, please could you test the attached patch (against py3k) with
    suncc? With this patch, sys.float_repr_style should report 'short', and
    'make test' should not produce any obviously float-related failures.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    My copy is 32-bit. I never installed a 64-bit version, but I strongly
    assume that uname -p gives x86_64. BTW, uname -p works on Solaris, but
    returns 'unknown' on my 64 bit Linux.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    Tested the patch against an updated 3.2. repr-style is 'short', and I
    did not see obvious float errors. In particular, test_float.py is
    passed. I also did not see new compile warnings.

    As for the other tests, the errors I get seem to be the same with or
    without the patch. Any other tests I should watch out for apart from
    test_float.py?

    @mdickinson
    Copy link
    Member Author

    Many thanks for testing this, Stefan! I'll tidy up the patch a little bit
    and add a Misc/NEWS entry, then commit this.

    @mdickinson
    Copy link
    Member Author

    As for the other tests, the errors I get seem to be the same with or
    without the patch. Any other tests I should watch out for apart from
    test_float.py?

    Well *ideally* you shouldn't be getting any test failures :-). But I
    could imagine than running Solaris within a VM might cause some
    problems. Which tests are failing?

    Tests I'd be worried about for the float repr change include:
    test_float, test_complex, test_math, test_cmath, test_ast, test_format,
    test_marshal, test_pickle, test_json, test_builtin, test_capi.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    The tests that you mention run o.k., except capi, but that looks harmless:

    stefan@opensolaris:~/svn/py3k/Lib/test# ../../python test_capi.py
    test_instancemethod (main.CAPITest) ... ok

    ----------------------------------------------------------------------
    Ran 1 test in 0.000s

    OK
    internal test_L_code
    internal test_Z_code
    internal test_capsule
    internalTraceback (most recent call last):
      File "test_capi.py", line 170, in <module>
        test_main()
      File "test_capi.py", line 130, in test_main
        print("internal", name)
    ImportError: No module named _curses

    Tests that fail:

    test_ascii_formatd, test_calendar, test_datetime, test_distutils,
    test_email, test_httpservers, test_mailbox, test_multiprocessing,
    test_os, test_pipes, test_platform, test_poll, test_popen, test_pty,
    test_pydoc, test_quopri, test_select, test_signal, test_strftime,
    test_strptime, test_subprocess, test_sys, test_tempfile, test_threading

    A lot of these fail either due to the inability to allocate resources or
    the fact that strftime appears to give weird results.

    @mdickinson
    Copy link
    Member Author

    Is the test_ascii_formatd failure due to a failed 'from ctypes import
    ...'? That's the only failure that looks like it could be related.

    Thanks for this.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Nov 9, 2009

    Yes, test_ascii_formatd fails with 'ImportError: No module named _ctypes'.

    @mdickinson
    Copy link
    Member Author

    Committed in r76300 (trunk) and r76301 (py3k). Thanks for all your help,
    Stefan.

    @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
    easy interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant