[Webkit-unassigned] [Bug 122627] DFG: Add JIT support for LogicalNot(String/StringIdent)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=122627


Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #213950|review?                     |review-
               Flag|                            |




--- Comment #2 from Filip Pizlo <fpizlo at apple.com>  2013-10-10 18:46:36 PST ---
(From update of attachment 213950)
View in context: https://bugs.webkit.org/attachment.cgi?id=213950&action=review

Almost good, just needs a few fixes.

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:573
>                  break;
> +            case StringUse:
>              case ObjectOrOtherUse:

I think that the StringUse case should be in the case block above, since the only reason why it would exit is for the type check.  AbstractInterpreter already will setCanExit(true) if it determines that the type check implied by the use kind is necessary.

> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:296
> +            else if (node->child1()->shouldSpeculateString() || node->child1()->shouldSpeculateStringIdent())
> +                fixEdge<StringUse>(node->child1());

shouldSpeculateString() should subsume shouldSpeculateStringIdent(), so you don't need the second part.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4170
> +void SpeculativeJIT::compileStringIdentZeroLength(Node* node)

See below about method name.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1663
> +    case StringUse:
> +        return compileStringIdentZeroLength(node);
> +

I wouldn't call this method compileStringIdentZeroLength.  It doesn't have anything to do with StringIdent - you're not requiring the operand to be an identifier, you're only requiring it to be a string.  So, a name like compileStringZeroLength would be fine.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.