Closed Bug 822265 Opened 10 years ago Closed 10 years ago

BaselineCompiler: JSOP_NEG with double

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

      No description provided.
Assignee: general → evilpies
Status: NEW → ASSIGNED
Attachment #692943 - Flags: review?(jdemooij)
Attached patch baseline patchSplinter Review
This is for ss-3d-morph. But we still need TypeUpdate and UseCount stubs.
Attachment #692945 - Flags: review?(jdemooij)
Comment on attachment 692943 [details] [diff] [review]
trunk patch for masm.negateDouble

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

::: js/src/ion/arm/MacroAssembler-arm.cpp
@@ +67,5 @@
>  
>  void
> +MacroAssemblerARM::negateDouble(FloatRegister reg)
> +{
> +    ma_vneg(input, input);

s/input/reg
Attachment #692943 - Flags: review?(jdemooij) → review+
Attachment #692945 - Attachment is patch: true
Comment on attachment 692945 [details] [diff] [review]
baseline patch

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

Cool, thanks for doing this! r=me with comments addressed

::: js/src/ion/BaselineIC.cpp
@@ +985,5 @@
>          // TODO: Discard/replace stubs.
>          return true;
>      }
>  
> +    if (val.isDouble() && res.isDouble() && op == JSOP_NEG) {

Remove this "if", the isNumber() case below handles it.

@@ +999,5 @@
> +        ICUnaryArith_Int32::Compiler compiler(cx, op);
> +        ICStub *int32Stub = compiler.getStub(ICStubSpace::StubSpaceFor(script));
> +        if (!int32Stub)
> +            return false;
> +        stub->addNewStub(int32Stub);

"return true;" here, so that we don't add two stubs.

@@ +1003,5 @@
> +        stub->addNewStub(int32Stub);
> +    }
> +
> +    if (val.isNumber() && res.isNumber() && op == JSOP_NEG) {
> +        ICUnaryArith_Double::Compiler compiler(cx, op);

Can you add a stub->unlinkStubsWithKind(ICStub::UnaryArith_Int32); here? The double stub handles both cases, so we should just use that, and it doesn't really matter for type information (Ion will specialize as double anyway).
Attachment #692945 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/5589176b4f58
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Forgot to put leave open in there.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/projects/ionmonkey/rev/38d60bed7b75
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.