[Patches] [ python-Patches-664320 ] Replace pop/push pairs in ceval.c

[SourceForge.net]

Patches item #664320, was opened at 2003-01-08 06:34
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=664320&group_id=5470

Category: Core (C code)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Raymond Hettinger (rhettinger)
Assigned to: Nobody/Anonymous (nobody)
Summary: Replace pop/push pairs in ceval.c

Initial Comment:
Paired PUSH() and POP() macros unnecessarily 
decrement and then re-increment the stack pointer.  
In these cases, indexed access to and from the stack 
is faster.

Saves the add 4 and sub 4 in the assembler code. 
Allows the compiler more freedom to optimize (by 
not overspecifying or creating sequential 
interdependencies). Lets the processor run more of 
the instructions in parallel (adjusting the index 
prevents parallel execution any move or other 
instruction that depends on the index).

I wouldn't begin to know how to time the savings 
here, but examining the  generated assembler shows 
that the code has improved.

----------------------------------------------------------------------

>Comment By: Neal Norwitz (nnorwitz)
Date: 2003-01-08 22:54

Message:
Logged In: YES 
user_id=33168

One thing to note, if LLTRACE is defined, you lose that
capability since PUSH/POP are not called that frequently.  I
don't know how useful LLTRACE is and I don't care about this
"loss."  mwh may have used it recently, so it may be good to
get his opinion.

I reviewed patch in detail and didn't find any problems.  I
ran the regression suite on Linux without problems.  I also
ran pychecker over the stdlib, that sometimes provokes weird
bugs, no problems there either. :-)

If anything, I like the code better after this patch. 
Definitely +1.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-01-08 22:33

Message:
Logged In: YES 
user_id=33168

I'm reviewing this patch now and have a question.  There is
code to handle DUP_TOPX for values of X == 4 and 5.  I don't
see how this can be generated.  I greped the code for
DUP_TOPX and only found values of 2 and 3.  Should the code
for 4 and 5 be removed?  Am I missing something?

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-01-08 13:37

Message:
Logged In: YES 
user_id=31435

I haven't reviewed this in detail (and won't), but +1 on the 
idea -- these kinds of small code improvements may or may 
not yield immediate speedups, but reliably move things in 
the right direction over time, as they compound.  The parts 
of the patch I did stare at didn't suffer in clarity, so +1 all 
around.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=664320&group_id=5470