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
HAVE_PY_SET_53BIT_PRECISION for m68k #65103
Comments
m68k has the same problem as x86 with excess floating point precision. The attached patch implements the necessary support for HAVE_PY_SET_53BIT_PRECISION. |
Technically I guess this counts as a new feature. It would be painful to have to wait for 3.5, though. I wonder whether we can sneak this in after 3.4 is released? Is the __mc68000__ #define specific to gcc? |
I don't know of any other compiler on m68k. |
Mark Dickinson <report@bugs.python.org> wrote:
+1. m68k affects a relatively small group of people, and Andreas Schwab is |
Does Python still officially support m68k? |
I have modified the patch to include a configure check to set HAVE_GCC_ASM_FOR_MC68881 and use that instead of __mc68000__. |
I don't think fixing bugs on a specific architecture counts as a new feature.
Certainly not. We haven't had any 68k buildbot in ages (not sure we ever had any, actually). Andreas, have you signed a contributor's agreement? You can do it online at http://www.python.org/psf/contrib/contrib-form/ |
I'm happy to accept the change for 3.4.1, but I'm not going to cherry-pick a fix for an unsupported platform after rc3. |
It strikes me as strange that we'd allow code churn for an unsupported platform, can someone explain the rationale behind this please. |
What do you mean with code churn? |
Code churn is defined as lines added, modified or deleted to a file from one version to another. |
That's a very broad definition, I didn't know that python is such a hostile environment. |
Ignore Mark Lawrence. |
@mark, I don't understand why you ask this question after several positive responses of Python committers (Mark, Stefan, Larry). @andreas, as far as I recall, we have always welcomed patches for officially unsupported platforms, certainly as long as they only introduce special code for that platform, as is the case here. |
I love you as well Benjamin :) To be serious I think we're talking at cross purposes. I'm not against this patch. Code churn often gets mentioned here as a reason for not doing what is proposed. Changing code for an unsupported platform just struck me as odd. So if somebody explains that we're doing it for the very good reasons x, y and z I'll be perfectly happy. |
So far there are only very few m68k patches. Additionally, the patches are well researched and sometimes highlight ANSI C violations. The submitters of the patches are highly competent and apparently take testing seriously. I see no reason to reject the patches. |
With respect, Mark, I think you should leave these considerations to the committers. |
Great, you ask a simple, straight forward question and get told to go away. I'll therefore leave everything to the committers, including the roughly 4500 open issues and the other 40 languishing. |
Since 3.4 and 3.5 are different code bases, I assume you'd be willing to check this in for both. Assuming that's the case, please tick the 3.5 version too. |
Mark, I already gave a reason, which was apparently not good enough for you. Nobody told you to go away from the tracker, but it's not constructive to ask for reasons why a particular patch is accepted or rejected, by the release manager no less, if you're not the original author. (Except if you want to discuss policy, in which case the tracker is the wrong place to do it.) |
I've asked a simple question and I've *NOT* had an answer. Antoine's response to Larry's question "Does Python still officially support m68k?" is certainly pertinent "Certainly not. We haven't had any 68k buildbot in ages (not sure we ever had any, actually)." The more I see here, the more laughable I think the situation is. Core devs have time to spend on an issue for an unsuppoorted platform, but don't have the time to support the 4500 issues that are presumably aimed at supported platforms. What gives? |
Can we move along, please? |
Indeed. Further discussion, if felt to be really necessary, should take place on python-dev. |
BreamoreBoy: Support for this patch comes from several properties of the patch and the way it is stated that make it easy to like it. It is well-researched, well-presented, and clearly can't have negative impact on the systems that *are* supported. This is different from the other thousands of issues which are much more difficult to rule on. There is, of course, the standing ruling from Guido that we shouldn't let new support for minority platforms in, and phase out any such existing support that is already in the code base. By that policy, Andreas would have to support his own fork of Python. |
Martin: I hadn't realized there was such a rule from Guido. Can you cite where he said that? Obviously I didn't mind accepting this patch, as it looks like it would have roughly zero maintenance cost for anyone who doesn't care about 68k. But if Guido has said "we don't accept patches for unsupported platforms", I may have to reverse my decision. |
It's not really a bugfix, though. Python 3.4 *should* (I'm not in a position to check, but Andreas may be) be behaving as designed on m68k: the configure script will correctly determine that there's a potential issue with double rounding, and since it doesn't currently know of any way to control the FPU precision setting on m68k, it'll set the environment variables up so that the legacy floating-point repr code is used. The built Python should function as normal, expect that sys.float_repr_style will be 'legacy' instead of 'short', and we won't get the (primarily cosmetic) benefits of the short float repr. This patch then changes the part where Python doesn't know how to change the precision, allowing it to use David Gay's short float repr code instead of the legacy code. So I see it as an enhancement rather than a bugfix. And this would actually be a somewhat significant behaviour change: on m68k with Python 3.4.0, we'd see: >>> 1.1
1.1000000000000001 and (if this patch went into the 3.4 branch), on Python 3.4.1 we'd see instead: >>> 1.1
1.1 The behaviour of string formatting and the round function would also change in edge cases. There's an argument that the number of users affected by this change is likely to be tiny, so changing this in 3.4.1 is unlikely to break people's code. But the tininess of the userbase is equally the basis of an argument that the change isn't at all urgent, and those affected can wait for Python 3.5 or patch their copy of Python; I don't see a really good reason to break the policy about new features on bugfix branches for this particular issue. Given all that, I don't think it would be appropriate to include the change in Python 3.4.1. I'd personally like to see it go into Python 3.5, but that's dependent on the outcome of the "we don't accept patches for unsupported platforms" discussion (which is orthogonal to the 'is this an enhancement or a bugfix' discussion). |
It's not just cosmetic, it's breaking the testsuite back and forth. |
Sure; those are really bugs in the tests, though: no test should be blindly assuming that the short float repr is in use. It sounds as though we're missing some skip decorators. |
In the absence of Guido here, I'll channel him ;-) "The problem" with oddball platforms has been that some require major changes in many parts of the interpreter, and then all the added cruft complicates life for every maintainer, while few people benefit and the oddball platform typically has only one maintainer who vanishes for long stretches. Guido would not object to this small, simple, well-motivated and isolated patch. At least he wouldn't object on the basis of "it's an unsupported platform". I don't object either ;-) |
It's a minor patch for a niche platform. What exactly is the point of |
It seems it wasn't actually a formal ruling (although I took it for that); see for yourself - or better, ask Guido what he thinks about this topic today: https://mail.python.org/pipermail/python-3000/2007-August/009692.html There might be more postings on the topic which I can't find now. |
What triggered my interpretation might have been this conversation: https://mail.python.org/pipermail/python-dev/2002-May/023998.html |
We are actually talking about Linux here, I assume everyone knows what that is :-) Also the patch is 2 files changed, 32+ (if you ignore the autoconf generated files), which is quite a bit smaller than the final version of the atheos patch (which is 19 files changed, 544+, 15-, also generated files ignored). |
Benjamin Peterson <report@bugs.python.org> wrote:
I think this is a very important point. Initially I was skeptical about m68k, So far, the m68k issues were about C-standard compliance and timing assumptions This one is a small patch that won't affect anything else. My experience with exotic Linux ports (Debian SPARC, etc.) is that the Python |
On Mon, Mar 24, 2014, at 14:56, Martin v. Löwis wrote:
In this case, though, the patch gets accepted: |
(And hooray for that, given the meteoric rise of AtheOS. :| ) I'm going to go way out on a limb and say that Guido hasn't made a pronouncement here. Also, the discussions cited by Martin are about entire new platforms (AtheOS, Haiku), whereas what we're talking about here is an additional architecture for an existing platform (m68k on linux). So I'm going to use my best judgement. I'm willing to accept the patch for 3.5, provided that:
I do have two questions:
Finally, Mark Dickinson is right: since this patch changes behavior in an incompatible way, it's not permissible to check it in for 3.4. Sorry. |
342 tests OK. |
Veto on m68k-float-prec.patch for Linux/m68k for now. Reasoning is same as in bpo-18062 (thanks skrah for linking it): Enabling this *will* break Python on Linux/m68k on the most The same applies to all other m68k OSes running in ARAnyM I think this could be applied when a released version of The problem here is that this *will* create a run-time issue. |
??? It will not of course, it will *fix* it. You have no idea what you are talking about. |
Andreas Schwab dixit:
No: it will break Debian/m68k which heavily uses Python, because:
bye,
|
There is no excuse for using a broken emulator. |
Andreas Schwab dixit:
Sure, if nobody releases a fixed version… and even then, I say that if you break ARAnyM you kill off Debian/m68k bye,
|
Finn Thain <fthain@telegraphics.com.au> writes:
Aranym *is* fixed. Andreas. |
Andreas Schwab dixit:
Yes, because of the value ARAnyM has for Linux/m68k development
Debian is consistent across architectures. (Well, mostly.) This
What *precise* version of ARAnyM is the first to have been released I never got any response to my message to upstream in which I asked Once we do have a fixed version, we can communicate that around. bye,
|
The fixed version is here: git://git.code.sf.net/p/aranym/code Andreas. |
If the asm instructions silently fail, I'd say add a test to ./configure Or don't bother and tell people to use the proper version of |
There is nothing that fails. The emulator has always correctly implemented the insn. |
Stefan Krah dixit:
No, the problem is at runtime: Debian is a binary distro, and thus, bye,
|
Andreas Schwab dixit:
That’s a source repository. I was asking for released tarballs But clearly I have been outvoted by the m68k porters. So please bye,
|
You don't have a "veto". Only Guido has that. Anyhow you have yet to reply to Mr. Schwab's assertion:
If that's true, then I don't understand what this whole argument is about. |
The only problem is that under some conditions involving denormalized numbers the result may lose a bit of precision. But that is mostly irrelevant for this issue, at least it wouldn't make it worse than it is now. |
That sounds like a non-issue for this application: the dtoa.c computations are careful to avoid subnormals in intermediate computations. If mirabilos has withdrawn his objection, is there anything blocking applying this for 3.5? |
Okay, I say let's check this in. If mirabilos can cite problems it causes we can revert it. Andreas, is there someone who would normally check this in for you, or should I do it? |
Larry Hastings <report@bugs.python.org> wrote:
Traditionally Mark commits the floating point stuff. :) |
New changeset c2f6551c9eaf by Benjamin Peterson in branch 'default': |
Yay! Thanks, Benjamin. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: