Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(8)

Issue 1994044: Change all stackops to use only allocas

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 11 months ago by ebo
Modified:
14 years, 11 months ago
Reviewers:
Reid Kleckner
CC:
unladen-swallow_googlegroups.com
Base URL:
http://unladen-swallow.googlecode.com/svn/trunk/
Visibility:
Public.

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+181 lines, -64 lines) Patch
M JIT/global_llvm_data.cc View 2 chunks +1 line, -5 lines 0 comments Download
M JIT/llvm_fbuilder.h View 3 chunks +12 lines, -0 lines 0 comments Download
M JIT/llvm_fbuilder.cc View 13 chunks +106 lines, -11 lines 1 comment Download
M JIT/opcodes/block.cc View 6 chunks +28 lines, -16 lines 1 comment Download
M JIT/opcodes/call.cc View 11 chunks +28 lines, -32 lines 0 comments Download
M JIT/opcodes/container.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M JIT/opcodes/control.cc View 2 chunks +2 lines, -0 lines 2 comments Download

Messages

Total messages: 2
ebo
This patch changes all stack operations to use allocas instead of writing to/reading from memory. ...
14 years, 11 months ago (2010-08-21 18:59:19 UTC) #1
Reid Kleckner
14 years, 11 months ago (2010-08-29 15:54:28 UTC) #2
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b