Bug 152165 – [JSC] Add an implementation of pow() taking an integer exponent to B3

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[JSC] Add an implementation of pow() taking an integer exponent to B3

[Benjamin Poulain]

Created attachment 267151 [details]
Patch

[Mark Lam]

Comment on attachment 267151 [details]
Patch

r=me.  Classic pow strength reduced to muls.  Nice.

[WebKit Commit Bot]

Comment on attachment 267151 [details]
Patch

Clearing flags on attachment: 267151

Committed r193989: <http://trac.webkit.org/changeset/193989>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Filip Pizlo]

Comment on attachment 267151 [details]
Patch

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

> Source/JavaScriptCore/b3/B3MathExtras.cpp:49
> +    BasicBlock* functionCallCase = procedure.addBlock();
> +    BasicBlock* loopPreHeaderCase = procedure.addBlock();
> +    BasicBlock* loopTestForEvenCase = procedure.addBlock();
> +    BasicBlock* loopOdd = procedure.addBlock();
> +    BasicBlock* loopEvenOdd = procedure.addBlock();
> +    BasicBlock* continuation = procedure.addBlock();

This breaks basic block ordering.  FTL creates all of the LLVM basic block entrypoints for DFG basic blocks, and then later it will insert basic blocks in between those other blocks so that all of the LLVM blocks associated with a DFG block are grouped together.  Right now, we have yet to port this feature from the LLVM version of Output to the B3 version.  But that does not mean that we will abandon the feature.  It's totally sensible for a compiler to assume that the incoming program order is appropriate for things like live ranges.

I don't see how to fix this given the restrictive API you've created.  Your API does not allow the caller to designate which block new blocks should be inserted before.  This is why FTL::Output works the way that it does.  Please remove the powDoubleInt32() function from the B3 namespace and move it to FTL::Output, so that once we port our basic block ordering code to B3, we can make it work with this.

> Source/JavaScriptCore/b3/B3MathExtras.h:41
> +// Raise "x" to "y" power.
> +// Return a new block continuing the flow and the value representing the result.
> +JS_EXPORT_PRIVATE std::pair<BasicBlock*, Value*> powDoubleInt32(Procedure&, BasicBlock*, Origin, Value* x, Value* y);

Let's not do this.  This belongs in FTLB3Output.  It's the wrong API, and it breaks the way that FTL orders basic blocks.  If you strongly feel that Pow(double, int) should be a B3 feature, then make it an opcode and handle it in LowerMacros.  But that doesn't seem like the right approach, since that just adds opcodes.

FTLOutput owns basic block ordering, which means that it has the power to implement an operation in terms of multiple basic blocks while still maintaining a sensible order.