[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