Change all stackops to use only allocas (issue1994044)

3 views
Skip to first unread message

e...@4geeks.de

unread,
Aug 21, 2010, 2:59:19 PM8/21/10
to e...@4geeks.de, unladen...@googlegroups.com, re...@codereview.appspotmail.com
Reviewers: ,

Message:
This patch changes all stack operations to use allocas instead of
writing to/reading from memory.
The stack is only written to memory if needed, which means before
bailing and exception handling and during some opcode implementations.

This gives a slight speed increase on all benchmarks (between 1.01 and
1.10 times faster) and will enable other more advanced optimizations.

Please review this at http://codereview.appspot.com/1994044/

Affected files:
M JIT/global_llvm_data.cc
M JIT/llvm_fbuilder.cc
M JIT/llvm_fbuilder.h
M JIT/opcodes/block.cc
M JIT/opcodes/call.cc
M JIT/opcodes/container.cc
M JIT/opcodes/control.cc


reid.k...@gmail.com

unread,
Aug 29, 2010, 11:54:28 AM8/29/10
to e...@4geeks.de, unladen...@googlegroups.com, re...@codereview.appspotmail.com
Sorry for the delay. Any perf results?


http://codereview.appspot.com/1994044/diff/1/8
File JIT/llvm_fbuilder.cc (right):

http://codereview.appspot.com/1994044/diff/1/8#newcode1284
JIT/llvm_fbuilder.cc:1284: Value *former_top =
this->builder_.CreateLoad(this->stack_[this->stack_top_]);
80

http://codereview.appspot.com/1994044/diff/1/6
File JIT/opcodes/block.cc (right):

http://codereview.appspot.com/1994044/diff/1/6#newcode231
JIT/opcodes/block.cc:231: this->fbuilder_->SaveAllToStack();
Doesn't the unwind block save things to the stack? Comment why the
SaveAll is needed, I can't actually tell.

http://codereview.appspot.com/1994044/diff/1/4
File JIT/opcodes/control.cc (right):

http://codereview.appspot.com/1994044/diff/1/4#newcode145
JIT/opcodes/control.cc:145: this->fbuilder_->SaveAllToStack();
Return also goes to the unwind block, which saves the stack. I'd
comment why this is needed if it is.

http://codereview.appspot.com/1994044/diff/1/4#newcode169
JIT/opcodes/control.cc:169: this->fbuilder_->SaveAllToStack();
ditto, comment below notwithstanding. We shouldn't be using anything
past the SP.

http://codereview.appspot.com/1994044/

Reply all
Reply to author
Forward
0 new messages