Bug 122081 – FTL: refactor compileAdd and compileArithSub into one function.

• Component: New Bugs
• Status: RESOLVED FIXED

[Nadav Rotem]

FTL: refactor compileAdd and compileArithSub into one function.

[Nadav Rotem]

Created attachment 212941 [details]
Patch

[Geoffrey Garen]

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.

[Nadav Rotem]

Created attachment 212947 [details]
Patch

[Geoffrey Garen]

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"?

[Nadav Rotem]

Okay. I will fix the 'O'

[Nadav Rotem]

Created attachment 212949 [details]
Patch

[Geoffrey Garen]

Comment on attachment 212949 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 212949 [details]
Patch

Clearing flags on attachment: 212949

Committed r156634: <http://trac.webkit.org/changeset/156634>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.