Bug 19750

Summary: Merge inc/dec halves of PrefixResolveNode and PostfixResolveNode
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: JavaScriptCoreAssignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Patch to merge inc/dec halves of PrefixResolveNode and PostfixResolveNode
zwarich: review-
No method pointers, fewer parentheses, fixed fixme. zwarich: review+

Gavin Barraclough
Reported 2008-06-24 09:16:02 PDT
Post Squirrelfish there is no longer a performance gain in having separate nodes for inc and dec – merging these will simplify & reduce code duplication in the parse tree. Patch following.
Attachments
Patch to merge inc/dec halves of PrefixResolveNode and PostfixResolveNode (13.28 KB, patch)
2008-06-24 09:16 PDT, Gavin Barraclough 		zwarich: review-
DetailsFormatted DiffDiff
No method pointers, fewer parentheses, fixed fixme. (13.48 KB, patch)
2008-06-25 08:13 PDT, Gavin Barraclough 		zwarich: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Gavin Barraclough
Comment 1 2008-06-24 09:16:44 PDT
Created attachment 21908 [details] Patch to merge inc/dec halves of PrefixResolveNode and PostfixResolveNode
Cameron Zwarich (cpst)
Comment 2 2008-06-24 21:29:36 PDT
Comment on attachment 21908 [details] Patch to merge inc/dec halves of PrefixResolveNode and PostfixResolveNode I think this is unnecessary: + RegisterID* (CodeGenerator::*emitPostIncOrDec)(RegisterID* dst, RegisterID* srcDst) = (m_operator == OpPlusPlus) ? &CodeGenerator::emitPostInc : &CodeGenerator::emitPostDec; + RegisterID* (CodeGenerator::*emitPreIncOrDec)(RegisterID* srcDst) = (m_operator == OpPlusPlus) ? &CodeGenerator::emitPreInc : &CodeGenerator::emitPreDec; Use CodeGenerator::emitUnaryOp() and CodeGenerator::emitUnaryNoDstOp() with an inline conditional expression, or make a helper function. You don't need parentheses around the entire conditional expression here: + RefPtr<RegisterID> r0 = generator.emitLoad(generator.finalDestination(dst), ((m_operator == OpPlusPlus) ? 1.0 : -1.0)); Also, rewrite the FIXME comment in PostfixResolveNode::emitCode() so it is still correct.
Gavin Barraclough
Comment 3 2008-06-25 08:13:17 PDT
Created attachment 21930 [details] No method pointers, fewer parentheses, fixed fixme. Cameron, I've replaced the method pointers with simple static helper methods in nodes.cpp – if you could have a look & see what you think that'd be great. cheers, G.
Cameron Zwarich (cpst)
Comment 4 2008-06-25 10:29:28 PDT
Comment on attachment 21930 [details] No method pointers, fewer parentheses, fixed fixme. r=me
Gavin Barraclough
Comment 5 2008-07-17 06:26:17 PDT
Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/kjs/grammar.y Sending JavaScriptCore/kjs/nodes.cpp Sending JavaScriptCore/kjs/nodes.h Sending JavaScriptCore/kjs/nodes2string.cpp Transmitting file data ..... Committed revision 35222.
Note You need to log in before you can comment on or make changes to this bug.