Summary: | Create fused opcodes for tests and conditional jumps | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Cameron Zwarich (cpst) <zwarich> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED WONTFIX | ||||||||
Severity: | Normal | CC: | ap, barraclough, ggaren, mjs, oliver, sam | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Cameron Zwarich (cpst)
2008-06-09 20:49:10 PDT
Created attachment 21598 [details]
Proposed patch
Here's the patch. One issue I see is that CodeGenerator::lastOpcodeID is uninitialized. I wasn't sure of the best way to do it, because OpcodeID is an enum. Any suggestions? The documentation for j_less is also wrong.
Created attachment 21613 [details]
Revised proposed patch
Maciej says that this method of doing peephole optimizations won't conflict with his plans for fusing loads into other operations, so here's a cleaned up version of the patch for review. I initialize m_lastOpcodeID to op_end, since no optimization will ever optimize a sequence starting with op_end.
We still might want to consider a different method of code generation that incorporates all of these optimizations, but we might as well test them out incrementally before we go rewriting everything.
Comment on attachment 21613 [details]
Revised proposed patch
395 void CodeGenerator::retrieveLastBinaryOp(int& dstIndex, int& src1Index, int& src2Index)
396 {
397 size_t size = instructions().size();
398 dstIndex = instructions().at(size - 3).u.operand;
399 src1Index = instructions().at(size - 2).u.operand;
400 src2Index = instructions().at(size - 1).u.operand;
401 }
This should assert that size is >= 4.
405 instructions().shrink(instructions().size() - 4);
403 void CodeGenerator::rewindBinaryOp()
404 {
405 instructions().shrink(instructions().size() - 4);
406 }
This too.
r=me
Landed in r34497. I will keep this bug open to discuss other possibilities for test / jump opcode fusion, but this was definitely the most common case. Comment on attachment 21613 [details]
Revised proposed patch
Clearing review flag.
Is this bug still actionable? Further bytecode superinstructions is not something we're likely to do right now. |