882925 - BaselineCompiler: ICUnaryArith_Double stubs should have UnaryArith_Double kind

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Kannan Vijayan [:djvj]]

From BaselineIC.cpp:

class ICUnaryArith_Double : public ICStub
{
    friend class ICStubSpace;

    ICUnaryArith_Double(IonCode *stubCode)
      : ICStub(UnaryArith_Int32, stubCode)
    {}
...

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: Fix.

Comment 2

[Hannes Verschore [:h4writer]]

Comment on attachment 762328 [details] [diff] [review]
Fix.

Review of attachment 762328 [details] [diff] [review]:
-----------------------------------------------------------------

Just saw you forgot another UnaryArith_Int32 in the ICMultiStubCompiler part of ICUnaryArith_Double. Please update this too ;)

Comment 3

[Kannan Vijayan [:djvj]]

Try run looks ok:
https://tbpl.mozilla.org/?tree=Try&rev=b1f8b5728f3e

Oranges are inherited from parent.

Pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2e1286ec3ed

Comment 4

[Hannes Verschore [:h4writer]]

Just for bookkeeping but this introduced a 5.7% regression on box2d. So for some reason the wrong information caused box2d to be faster. Interesting ...

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d2e1286ec3ed

Comment 6

[Kannan Vijayan [:djvj]]

(In reply to Hannes Verschore [:h4writer] from comment #4)
> Just for bookkeeping but this introduced a 5.7% regression on box2d. So for
> some reason the wrong information caused box2d to be faster. Interesting ...

Could you make a bug for that?  This is likely an issue where we wrongly specialize ion code to integers, and then bail when doubles show up.  There may be long runs of integers that span past the 1000 use count limit before compiling to Ion.

The solution might be to remember the overflow bit in the fallback stub instead of just scanning the optimized stubs.  That way a GC can't wipe out that info.