[issue4715] optimize bytecode for conditional branches

[Jeffrey Yasskin]

Jeffrey Yasskin <jyasskin at gmail.com> added the comment:

Looking through the patch...

I don't really like the names for JUMP_OR_POP and POP_OR_JUMP. (They
don't really indicate to me that the choice depends on the truth of the
top of the stack.) How about JUMP_IF_FALSE_OR_POP and
JUMP_IF_TRUE_OR_POP (or s/OR/ELSE/)? 

If the old opcodes weren't called JUMP_IF_XXX, I'd suggest the
always-popping variants just use those names. Many other opcodes
implicitly consume their arguments so these could too. But since this
would be confusing with both the old meanings and the other new pair,
your names are probably better.

The comments in opcode.h and opcode.py are inconsistent.

I now get a warning that PRED_POP_TOP is defined but not used, so you
should remove the PREDICTED macro for it.

I wonder if BINARY_AND and BINARY_OR should get predictions ... not for
this patch.

I would probably put the POP_JUMP_IF_XXX definitions next to the
JUMP_OR_POP definitions to emphasize their similarity.

You missed a comment referring to JUMP_IF_FALSE before
compiler_try_except().

POP_JUMP_IF_TRUE is only used in one place: assertions. I wonder if
anyone would cry if we compiled assertions to UNARY_NOT;
POP_JUMP_IF_FALSE instead... Anyway, also not for this patch.

In compiler_comprehension_generator, "compiler_use_next_block(c, skip);"
is now always followed by "compiler_use_next_block(c, if_cleanup);".
Should you remove the use(skip) call?

In peephole.c, s/JUMP_SIGN/JUMPS_ON_TRUE/ ?

test_peepholer fails for me, which makes sense because it still refers
to JUMP_IF_XXX.

Whoo, the peephole.c changes are complex. I'll do those in a second round.

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue4715>
_______________________________________