|
|
|
Created:
16 years, 7 months ago by mcquillan.sean Modified:
16 years, 7 months ago CC:
unladen-swallow_googlegroups.com Base URL:
http://unladen-swallow.googlecode.com/svn/trunk/ Visibility:
Public. |
Patch Set 1 #
Total comments: 4
Total messages: 4
|
mcquillan.sean
|
16 years, 7 months ago (2009年06月15日 23:45:17 UTC) #1 | ||||||||||||||||||||||||||||
Thanks for the patch! As I mentioned in http://code.google.com/p/unladen-swallow/issues/detail?id=42#c4, we'll eventually want to represent these constants with llvm::ConstantStructs, but this is a good start. The ConstantStructs will only have a big effect after we inline PyNumber_Add(). http://codereview.appspot.com/74058/diff/1/3 File Python/llvm_fbuilder.cc (right): http://codereview.appspot.com/74058/diff/1/3#newcode827 Line 827: Value *const_ = this->builder_.CreateLoad(this->consts_tup[index]); I believe this should be Value *const_ = ConstantExpr::getIntToPtr( ConstantInt::get( Type::Int64Ty, reinterpret_cast<uintptr_t>(PyTuple_GET_ITEM(co_consts, idx))), PyTypeBuilder<PyObject*>::get()); The code you have removes a load from the stack and an add (GEP lowers to an add to a register, but since that register's virtual and lives through the entire function, we should assume it will have been spilled), but leaves the load from the heap (the consts tuple). You really want to remove the load from the heap too. ConstantExprs (and more generally, all Constants) in LLVM are uniqued but then never destroyed, so you get memory use proportional to the total number and complexity of constants you ever use. Instructions are owned by the function they appear in and deleted when they're no longer used. http://codereview.appspot.com/74058/diff/1/3#newcode2298 Line 2298: //create Value<PyObject**> array of &(co_consts->ob_item[i]). PyObject** Please put a space after all "//"s and capitalize your comments. http://codereview.appspot.com/74058/diff/1/3#newcode2301 Line 2301: for(Py_ssize_t idx = 0; idx < sz; idx += 1) Control structures get a space between the keyword and the opening '('. The brace also goes on the same line as the closing ')', not on a separate line. We only use the separate line convention for functions. http://codereview.appspot.com/74058/diff/1/2 File Python/llvm_fbuilder.h (right): http://codereview.appspot.com/74058/diff/1/2#newcode422 Line 422: llvm::OwningArrayPtr<llvm::Value *> consts_tup; Member variables end in a '_'.
I've cleaned up Sean's patch and put it in http://codereview.appspot.com/75077, which contains performance results and before/after IR comparisons. If you can look it over, Jeff, I'll go ahead and commit it.
On 2009年06月17日 14:48:11, Collin Winter wrote: > I've cleaned up Sean's patch and put it in http://codereview.appspot.com/75077, > which contains performance results and before/after IR comparisons. If you can > look it over, Jeff, I'll go ahead and commit it. Committed as r650. Thanks for the patch, Sean!