Bug 113646 – fourthTier: FTL JIT should be able to compile the Array.prototype.findGraphNode function in Kraken/ai-astar

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

It's a super easy function to do compete type inference on and so it doesn't rely on either slow path calls or OSR exits.  Compiling it is just a matter of filling in more opcodes.

Patch forthcoming.

[Filip Pizlo]

Created attachment 195879 [details]
work in progress

[Filip Pizlo]

Created attachment 195884 [details]
the patch

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/147286

[Sam Weinig]

Comment on attachment 195884 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195884&action=review

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:777
> +    LValue lowObject(Edge edge, OperandSpeculationMode mode = AutomaticOperandSpeculation)

I assume "low" here means lower.  Why are you abbreviating it?  Or does it mean something else?

[Filip Pizlo]

(In reply to comment #4)
> (From update of attachment 195884 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195884&action=review
> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:777
> > +    LValue lowObject(Edge edge, OperandSpeculationMode mode = AutomaticOperandSpeculation)
> 
> I assume "low" here means lower.  Why are you abbreviating it?  Or does it mean something else?

Three reasons:

1) It doesn't really mean lower; it usually means "give me the value in a low-level form".  To me "lowering" is the act of compiling an instruction to a lower-level form; "lowObject()" doesn't compile any instructions, it mostly just looks up the LLVMValueRef corresponding to the result of a separately compiled (i.e. lowered) Node*.  Even if lowObject(), lowCell(), lowInt32(), and friends do some action before returning a value, that action is a check and not a lowering of the value itself.  More likely if they do some action, it'll involve a boxing conversion.  Hence the imperative "lower" is less applicable than the declarative "low".  The most appropriate name might have been "loweredObject()", but even that doesn't really capture what is going on.  Probably the full name would be "speculateObjectAndGetLoweredValue" but that starts to get pretty absurd.

2) lowInt32(node) makes sense to me as a getter: 'node' intrinsically already has an int32 representation even if we haven't generated code to create it yet (because we tend to do so lazily), and this is a getter that gives us the LLVM version (i.e. the "low level" version, abbreviated to "low") of the DFG version (i.e. the "high level" version, abbreviated in this code to "high" in other places) of the value.

3) I've come to the conclusion that the verbose style of the DFG does more harm than good.  It usually means that the code looks more intense than it really is.  I've made an effort to reduce the amount of typing required in the DFG itself in past patches.  In the FTL, I'm taking the approach I've taken before in compiler work: make the common things involve less typing.  This is why I call it "lowInt32" instead of "loweredInt32" or "speculateObject" or "speculateObjectAndGetLoweredValue".

To put it more succinctly, throughout the FTL lowering code I use "low" to mean the already-lowered form of something, and "high" to mean that original DFG form of that same something.  We have things like m_highBlock and m_lowBlock.  Those are fields because we create the lowered BasicBlocks eagerly.  For Node*, we cannot create the corresponding LLVMValueRefs eagerly (that would be illegal due to how LLVM constructs SSA), so we have lazy getters for retrieving the low value (the LLVMValueRef) for the high value (Node*).  But even though it's lazy rather than eager, I stick to the "low" versus "high" naming, and reserve the word "lower" to be a synonym for "compile to a lower-level representation".  Since doing lowObject(blah) doesn't compile blah, it doesn't make sense to call it lowerObject(blah).  Not to mention that I'm not sure what it means to "compile" an object.  You can compile, or lower, a node - that makes sense - but compiling or lowering a value doesn't make sense to me.