Closed Bug 882925 Opened 8 years ago Closed 8 years ago

BaselineCompiler: ICUnaryArith_Double stubs should have UnaryArith_Double kind

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: djvj, Unassigned)

References

Details

Attachments

(1 file)

From BaselineIC.cpp:

class ICUnaryArith_Double : public ICStub
{
    friend class ICStubSpace;

    ICUnaryArith_Double(IonCode *stubCode)
      : ICStub(UnaryArith_Int32, stubCode)
    {}
...
Attached patch Fix.Splinter Review
Attachment #762328 - Flags: review?(hv1989)
Attachment #762328 - Flags: review?(hv1989) → review+
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 ;)
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 ...
https://hg.mozilla.org/mozilla-central/rev/d2e1286ec3ed
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(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.
You need to log in before you can comment on or make changes to this bug.