classification
Title: Use computed gotos by default
Type: performance Stage: resolved
Components: Build, Interpreter Core Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pitrou Nosy List: jyasskin, lemburg, loewis, pitrou
Priority: normal Keywords:

Created on 2010-07-08 17:35 by pitrou, last changed 2010-08-13 22:26 by pitrou. This issue is now closed.

Messages (9)
msg109558 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-07-08 17:35
Now that the option has probably been extensively tested, it would be nice to enable computed gotos by default on systems that support them.
Perhaps this needs a dedicated test in the configure script.
msg109605 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2010-07-08 21:09
Antoine Pitrou wrote:
> 
> New submission from Antoine Pitrou <pitrou@free.fr>:
> 
> Now that the option has probably been extensively tested, it would be nice to enable computed gotos by default on systems that support them.
> Perhaps this needs a dedicated test in the configure script.

I doubt that it has been tested enough, but enabling it
for those platform/compiler where we know it works, should be
fine.

Have you tried testing this using the buildbots ?
msg109609 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2010-07-08 21:20
Marc-Andre Lemburg wrote:
> 
> Marc-Andre Lemburg <mal@egenix.com> added the comment:
> 
> Antoine Pitrou wrote:
>>
>> New submission from Antoine Pitrou <pitrou@free.fr>:
>>
>> Now that the option has probably been extensively tested, it would be nice to enable computed gotos by default on systems that support them.
>> Perhaps this needs a dedicated test in the configure script.
> 
> I doubt that it has been tested enough, but enabling it
> for those platform/compiler where we know it works, should be
> fine.

Here's a compiler/platform combination where it should not be used
per default:

gcc 4.4.2 on AMD processors ...
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42621

The fix will go into gcc 4.5, the report doesn't say whether it was
also backported to 4.4.x.
msg109612 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-07-08 21:28
> gcc 4.4.2 on AMD processors ...
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42621

So? Compilers have all kinds of optimization bugs, it's not our job to
have a compiler matrix with optimal flags per compiler.

(well, you can of course maintain such a matrix if you want)

For the record, I had an Athlon 64 machine until very recently, used it
with gcc 4.4.x and computed gotos were better than without.
msg109618 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2010-07-08 22:09
Antoine Pitrou wrote:
> 
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
>> gcc 4.4.2 on AMD processors ...
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42621
> 
> So? Compilers have all kinds of optimization bugs, it's not our job to
> have a compiler matrix with optimal flags per compiler.

No, but we do need to make sure that the casual user does not
run into such issues by using the default compiler on a typical
Unix system with Python default settings.

An expert user can always use the right compiler... but then an
expert can also use the configure option :-)

gcc 4.4.2 is not exactly an old version of gcc. The perspective
of (apparently) not having the patch in in Ubuntu 10.4 LTS's
default compiler suggests that we do need be more careful about
enabling the switch per default.

> (well, you can of course maintain such a matrix if you want)
> 
> For the record, I had an Athlon 64 machine until very recently, used it
> with gcc 4.4.x and computed gotos were better than without.

Perhaps someone can report whether this affects Ubuntu 10.4 LTS ?!
msg109623 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-07-08 22:24
> No, but we do need to make sure that the casual user does not
> run into such issues by using the default compiler on a typical
> Unix system with Python default settings.

The point is that we cannot make it sure. There is nothing specific
about computed gotos here: any kind of compiler bug could threaten
Python performance without us being able to do anything about it. Even
if we restrict ourselves to standard C features.

(also, I'll point out that it's not demonstrated that the problem above
affects Python, rather than simply the particular piece of code shown in
the bugzilla report)
msg109633 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-07-08 22:42
> No, but we do need to make sure that the casual user does not
> run into such issues by using the default compiler on a typical
> Unix system with Python default settings.

I think there is no risk here. The casual Python user on an AMD64 system
where gcc is the default compiler will use the Python installation that
comes with the operating system.
msg109690 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2010-07-09 08:44
Martin v. Löwis wrote:
> 
> Martin v. Löwis <martin@v.loewis.de> added the comment:
> 
>> No, but we do need to make sure that the casual user does not
>> run into such issues by using the default compiler on a typical
>> Unix system with Python default settings.
> 
> I think there is no risk here. The casual Python user on an AMD64 system
> where gcc is the default compiler will use the Python installation that
> comes with the operating system.

I'd be fine with enabling the switch on those platforms as well,
if we could get feedback from someone testing the behavior of gcc
on an AMD platform running Ubuntu 10.4.

If the problem is really only with the code snippet listed in the
bug report, there shouldn't be a problem and we can go ahead with
enabling computed gotos on that platform/compiler combination as well.
msg113832 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-13 21:16
I've committed a patch in r83986. I will watch the buildbots and close if everything's fine.
History
Date User Action Args
2010-08-13 22:26:56pitrousetstatus: pending -> closed
2010-08-13 21:16:59pitrousetstatus: open -> pending
messages: + msg113832

assignee: pitrou
resolution: fixed
stage: needs patch -> resolved
2010-07-09 08:44:51lemburgsetmessages: + msg109690
2010-07-08 22:42:57loewissetmessages: + msg109633
2010-07-08 22:24:25pitrousetmessages: + msg109623
2010-07-08 22:09:52lemburgsetmessages: + msg109618
2010-07-08 21:28:45pitrousetmessages: + msg109612
2010-07-08 21:20:52lemburgsetmessages: + msg109609
2010-07-08 21:09:50lemburgsetmessages: + msg109605
2010-07-08 17:35:03pitroucreate