Math.ceil() is useful in various tight computational kernels and should be inlined. The self hosted JS version (possibly incomplete) is roughly 3.5x faster. Measure: Math.ceil() Count: 1024 Elapsed: 2934.0000 (2.8652) Measure: ceil() Count: 1024 Elapsed: 850.0000 (0.8301)

On my computer: - with your ceil(): 538 - a local patch which only inlines the call to math_ceil_impl (without doing any inlining in assembly like MFloor does for Math.floor()): 305 There might still be room for improvement if we inline it directly in assembly.

(In reply to Benjamin Bouvier [:bbouvier] from comment #1) > - a local patch which only inlines the call to math_ceil_impl (without doing > any inlining in assembly like MFloor does for Math.floor()): 305 Do you happen to still have that patch? Inlining is faster but that's still much better than what we're doing now :)

Jan, here it is. It only inlines for the case where arg is a double and the return value is a double, which is hit in this test case. Feel free to review it if you think it's worth it :) Inlining in the other case (input is a double and returned value is an int32) shouldn't be hard to do: some factoring would be needed to avoid too much code duplication with MFloor. I thought we could use the fact that for all non-integers x, ceil(x) == floor(x) + 1 (that still holds for +/- Infinity and NaN), but x being a MIRType_Double doesn't make it necessarily a non-int. I will work on that probably later, unless somebody beats me to it :)

Comment on attachment 8345331 [details] [diff] [review] Inline ceil in some cases Review of attachment 8345331 [details] [diff] [review]: ----------------------------------------------------------------- Awesome, thanks!

I set leave open as we'd like some asm inlining too for the other case. https://hg.mozilla.org/integration/mozilla-inbound/rev/e3e21c3ada8c

When investigating the trigonometrics interpolations made for cos / sin in v8's math.js file, I saw that identity stating that Math.ceil == -Math.floor(-x). Fair enough, here's a free optimization for the last missing inlining case.

Comment on attachment 8375610 [details] [diff] [review] Missing cases Review of attachment 8375610 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but can you add a jit-test like this: function ceil(x) { return Math.ceil(x); } assertEq(ceil(1.1), 2); assertEq(ceil(-1.1), -1); assertEq(ceil(-3), -3); assertEq(ceil(-0), -0); We should Ion compile this function with --ion-eager and inline the call. Then the last call should trigger a bailout, right? Please verify this is what happens. r=me with that.

That's right, this is what's happening with your test case. I have also added cases for x == 0, since if Math.ceil is inlined as the Double -> Int32 variant (and thus uses the Floor trick), MFloor will bail out as it sees -0. Also added float32 tests for sanity. https://tbpl.mozilla.org/?tree=Try&rev=085275ae0185

Try looks good (orange is unrelated). https://hg.mozilla.org/integration/mozilla-inbound/rev/fefba9d06e03