BaselineCompiler: ICUnaryArith_Double stubs should have UnaryArith_Double kind

RESOLVED FIXED in mozilla24

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: djvj, Unassigned)

Tracking

unspecified
mozilla24
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

From BaselineIC.cpp:

class ICUnaryArith_Double : public ICStub
{
    friend class ICStubSpace;

    ICUnaryArith_Double(IonCode *stubCode)
      : ICStub(UnaryArith_Int32, stubCode)
    {}
...
Posted 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: 6 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.