Change the JIT to produce mostly register-machine based code. (issue1905048)

[e...@4geeks.de]

Reviewers: ,

Message:
This patch converts most of the JIT to use absolute stack values,
The only exception are exceptions or better block unwinding. I plan to
change this with the next patch.

These changes pass all tests and are performance neutral. Stack values
are still stored in a C array and can not be optimized efficiently.
Changing this to something similiar to locals would be the next step.

Please review this at http://codereview.appspot.com/1905048/show

Affected files:
M JIT/PyBytecodeIterator.cc
M JIT/PyBytecodeIterator.h
M JIT/llvm_compile.cc
M JIT/llvm_fbuilder.cc
M JIT/llvm_fbuilder.h
M JIT/opcodes/attributes.cc
M JIT/opcodes/attributes.h
M JIT/opcodes/binops.cc
M JIT/opcodes/call.cc
M JIT/opcodes/cmpops.cc
M JIT/opcodes/globals.cc
M Python/compile.c

[alex....@gmail.com]

My first thoughts are it mostly looks sane, but it's 7AM here... is
there anyway we could be testing this properly?

http://codereview.appspot.com/1905048/diff/2001/3001
File Python/compile.c (right):

http://codereview.appspot.com/1905048/diff/2001/3001#newcode679
Python/compile.c:679: opcode_stack_effect(int opcode, int oparg)
If this isn't static it needs an _Py prefix.

http://codereview.appspot.com/1905048/show

[reid.k...@gmail.com]

http://codereview.appspot.com/1905048/diff/7001/8012
File JIT/llvm_fbuilder.cc (right):

http://codereview.appspot.com/1905048/diff/7001/8012#newcode32
JIT/llvm_fbuilder.cc:32: #include <iostream>
I don't think this is needed.

http://codereview.appspot.com/1905048/diff/7001/8012#newcode66
JIT/llvm_fbuilder.cc:66: static int
Comment what this is doing.

http://codereview.appspot.com/1905048/diff/7001/8012#newcode71
JIT/llvm_fbuilder.cc:71: if (stack_info[iter.CurIndex()] >= 0) {
Comment that this is checking that if the stack level for this opcode
has been initialized, then we check that we got the same stack level
from both control flow paths and then cut off the recursion.

http://codereview.appspot.com/1905048/diff/7001/8012#newcode72
JIT/llvm_fbuilder.cc:72: assert(stack_level ==
stack_info[iter.CurIndex()]);
This assert seems like it could fail on malformed bytecode, so I'd
rather set a SystemExc and return an error code to avoid crashing.

http://codereview.appspot.com/1905048/diff/7001/8012#newcode1254
JIT/llvm_fbuilder.cc:1254: this->builder_.CreateStore(new_stack_pointer,
this->stack_pointer_addr_);
80

http://codereview.appspot.com/1905048/diff/7001/8002
File JIT/llvm_fbuilder.h (right):

http://codereview.appspot.com/1905048/diff/7001/8002#newcode151
JIT/llvm_fbuilder.h:151: /// This method reads on the direction of the
growing stack.
It's important that this line is clear. I'd write this as:
"Increasing argument offsets read in the direction of stack growth."

http://codereview.appspot.com/1905048/diff/7001/8002#newcode154
JIT/llvm_fbuilder.h:154: /// May only be called once per result per
IR-codepath.
s/May/Must/

http://codereview.appspot.com/1905048/diff/7001/8006
File JIT/opcodes/attributes.cc (right):

http://codereview.appspot.com/1905048/diff/7001/8006#newcode402
JIT/opcodes/attributes.cc:402: if (val_v != NULL) {
I think I understand why this is here now. This is the bail bb, so we
need to make the stack look like it did before the opcode started.

I don't think we really need these sets given that nothing should be
touching the Python stack between when we read it and when we bail.
However, we do need to restore the stack pointer to its previous value
before bailing.

I guess I wouldn't change anything now, but in the future it might be
worth keeping stack depth data around in the code object so that we can
restructure our opcodes to not modify the stack or stack pointer until
they have passed all their guards.

That way our guards and bail code can be as short as possible. Instead
of having every opcode undo its pushes, pops, and sets, the common bail
code can simply reset the stack pointer by looking up the depth in a
table in the code object.

http://codereview.appspot.com/1905048/diff/7001/8004
File JIT/opcodes/call.cc (left):

http://codereview.appspot.com/1905048/diff/7001/8004#oldcode271
JIT/opcodes/call.cc:271: this->fbuilder_->stack_pointer_addr());
This is a nice cleanup!

http://codereview.appspot.com/1905048/show

[abb...@gmail.com]

http://codereview.appspot.com/1905048/diff/7001/8012
File JIT/llvm_fbuilder.cc (right):

http://codereview.appspot.com/1905048/diff/7001/8012#newcode80
JIT/llvm_fbuilder.cc:80: stack_level = 1000;
What's this doing? Could it use a comment?

http://codereview.appspot.com/1905048/show