Summary: | FTL: refactor compileAdd and compileArithSub into one function. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nadav Rotem <nrotem> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Nadav Rotem
2013-09-29 18:45:00 PDT
Created attachment 212941 [details]
Patch
Comment on attachment 212941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212941&action=review > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:285 > case ValueAdd: > - compileAdd(); > + compileAddSub(false /* this is an add */); > break; > case ArithSub: > - compileArithSub(); > + compileAddSub(true /* this is a sub */); WebKit style for booleans that are "true" or "false" constants at the call site is to use named enum values. Something like: enum AddOrSub { Add, Sub }; compileAddOrSub(Add); compileAddOrSub(Sub); > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:696 > + LValue C1 =lowDouble(m_node->child1()); > + LValue C2 =lowDouble(m_node->child2()); Need spaces after the "="s. > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:707 > - > + Please revert this accidental whitespace change. Created attachment 212947 [details]
Patch
Comment on attachment 212947 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=212947&action=review > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:660 > + void compileAddSub(AddOrSubKind OpKind) > { > + bool isSub = OpKind == Sub; Sorry to keep going with the style nits, but the "O" in "OpKind" should be lower case. I'm going to r+ this patch. Can you post a follow-up to fix the "O"? Okay. I will fix the 'O' Created attachment 212949 [details]
Patch
Comment on attachment 212949 [details]
Patch
r=me
Comment on attachment 212949 [details] Patch Clearing flags on attachment: 212949 Committed r156634: <http://trac.webkit.org/changeset/156634> All reviewed patches have been landed. Closing bug. |