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+

Description Gavin Barraclough 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.
Comment 1 Gavin Barraclough 2008-06-24 09:16:44 PDT
Created attachment 21908 [details]
Patch to merge inc/dec halves of PrefixResolveNode and PostfixResolveNode
Comment 2 Cameron Zwarich (cpst) 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.
Comment 3 Gavin Barraclough 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.
Comment 4 Cameron Zwarich (cpst) 2008-06-25 10:29:28 PDT
Comment on attachment 21930 [details]
No method pointers, fewer parentheses, fixed fixme.

r=me
Comment 5 Gavin Barraclough 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.