[webkit-reviews] review requested: [Bug 20286] Load constants all at once instead of using op_load : [Attachment 22672] Proposed patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Aug 5 20:06:02 PDT 2008
Cameron Zwarich (cpst) <cwzwarich at uwaterloo.ca> has asked for review:
Bug 20286: Load constants all at once instead of using op_load
https://bugs.webkit.org/show_bug.cgi?id=20286
Attachment 22672: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=22672&action=edit
------- Additional Comments from Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>
Here's the patch. Here are a few comments:
1) I don't like that I have the same code
m_codeBlock->numConstants = programNode->numConstants() + 3;
in each of the CodeGenerator constructors, without any explanation of what it
does. Should I have a comment explaining why we possibly need 3 more registers
in each spot, or write it once and refer to it twice?
2) The way that I make some RegisterIDs in the temporary range return false for
isTemporary() seems like a bad idea, but since there is no copy constructor,
the only other way I can do it is by inheriting. What should I do here?
3) I changed CodeBlock::registers to be CodeBlock::constants, but perhaps the
name constantRegisters is better. What do you think?
More information about the webkit-reviews
mailing list