Bug 803730 - Fix boxing of object input in instanceOf checks. r=sstangl,dvander
authorKannan Vijayan <kvijayan@mozilla.com>
Mon, 12 Nov 2012 19:40:18 -0500
changeset 113041 e08357289bedc9c2ba253c7d5b3b82ac40402624
parent 113040 e12e84a82fa34d7da870b700c04ea7bc09a08c18
child 113042 c958c734e3d3580033d33fb406db26e53c646c94
push id23848
push useremorley@mozilla.com
push dateTue, 13 Nov 2012 16:29:34 +0000
treeherdermozilla-central@d56d537a1843 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssstangl, dvander
bugs803730
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 803730 - Fix boxing of object input in instanceOf checks. r=sstangl,dvander
js/src/ion/CodeGenerator.cpp
js/src/ion/arm/CodeGenerator-arm.cpp
js/src/ion/arm/CodeGenerator-arm.h
js/src/ion/arm/MacroAssembler-arm.cpp
js/src/ion/arm/MacroAssembler-arm.h
js/src/ion/x64/CodeGenerator-x64.cpp
js/src/ion/x64/CodeGenerator-x64.h
js/src/ion/x64/MacroAssembler-x64.h
js/src/ion/x86/CodeGenerator-x86.cpp
js/src/ion/x86/CodeGenerator-x86.h
js/src/ion/x86/MacroAssembler-x86.h
--- a/js/src/ion/CodeGenerator.cpp
+++ b/js/src/ion/CodeGenerator.cpp
@@ -4015,27 +4015,37 @@ CodeGenerator::emitInstanceOf(LInstructi
     Register rhsTmp = ToRegister(ins->getTemp(1));
     Register output = ToRegister(ins->getDef(0));
 
     // This temporary is used in other parts of the code.
     // Different names are used so the purpose is clear.
     Register rhsFlags = ToRegister(ins->getTemp(0));
     Register lhsTmp = ToRegister(ins->getTemp(0));
 
-    Label callHasInstance;
     Label boundFunctionCheck;
     Label boundFunctionDone;
     Label done;
     Label loopPrototypeChain;
 
+    JS_ASSERT(ins->isInstanceOfO() || ins->isInstanceOfV());
+    bool lhsIsValue = ins->isInstanceOfV();
+
     typedef bool (*pf)(JSContext *, HandleObject, HandleValue, JSBool *);
     static const VMFunction HasInstanceInfo = FunctionInfo<pf>(js::HasInstance);
 
-    OutOfLineCode *call = oolCallVM(HasInstanceInfo, ins, (ArgList(), rhs, ToValue(ins, 0)),
-                                   StoreRegisterTo(output));
+    // If the lhs is an object, then the ValueOperand that gets sent to
+    // HasInstance must be boxed first.  If the lhs is a value, it can
+    // be sent directly.  Hence the choice between ToValue and ToTempValue
+    // below.  Note that the same check is done below in the generated code
+    // and explicit boxing instructions emitted before calling the OOL code
+    // if we're handling a LInstanceOfO.
+
+    OutOfLineCode *call = oolCallVM(HasInstanceInfo, ins,
+        (ArgList(), rhs, lhsIsValue ? ToValue(ins, 0) : ToTempValue(ins, 0)),
+        StoreRegisterTo(output));
     if (!call)
         return false;
 
     // 1. CODE FOR HASINSTANCE_BOUND_FUNCTION
 
     // ASM-equivalent of following code
     //  boundFunctionCheck:
     //      if (!rhs->isFunction())
@@ -4047,17 +4057,28 @@ CodeGenerator::emitInstanceOf(LInstructi
 
     masm.mov(rhs, rhsTmp);
 
     // Check Function
     masm.bind(&boundFunctionCheck);
 
     masm.loadBaseShape(rhsTmp, output);
     masm.cmpPtr(Address(output, BaseShape::offsetOfClass()), ImmWord(&js::FunctionClass));
-    masm.j(Assembler::NotEqual, call->entry());
+    if (lhsIsValue) {
+        // If the input LHS is a value, no boxing necessary.
+        masm.j(Assembler::NotEqual, call->entry());
+    } else {
+        // If the input LHS is raw object pointer, it must be boxed before
+        // calling into js::HasInstance.
+        Label dontCallHasInstance;
+        masm.j(Assembler::Equal, &dontCallHasInstance);
+        masm.boxNonDouble(JSVAL_TYPE_OBJECT, ToRegister(ins->getOperand(0)), ToTempValue(ins, 0));
+        masm.jump(call->entry());
+        masm.bind(&dontCallHasInstance);
+    }
 
     // Check Bound Function
     masm.loadPtr(Address(output, BaseShape::offsetOfFlags()), rhsFlags);
     masm.and32(Imm32(BaseShape::BOUND_FUNCTION), rhsFlags);
     masm.j(Assembler::Zero, &boundFunctionDone);
 
     // Get Bound Function
     masm.loadPtr(Address(output, BaseShape::offsetOfParent()), rhsTmp);
@@ -4081,17 +4102,17 @@ CodeGenerator::emitInstanceOf(LInstructi
     //      output = true;
     //      goto done;
     //    }
     //  }
 
     // When lhs is a value: The HasInstance for function objects always
     // return false when lhs isn't an object. So check if
     // lhs is an object and otherwise return false
-    if (ins->isInstanceOfV()) {
+    if (lhsIsValue) {
         Label isObject;
         ValueOperand lhsValue = ToValue(ins, LInstanceOfV::LHS);
         masm.branchTestObject(Assembler::Equal, lhsValue, &isObject);
         masm.mov(Imm32(0), output);
         masm.jump(&done);
 
         masm.bind(&isObject);
         Register tmp = masm.extractObject(lhsValue, lhsTmp);
@@ -4120,17 +4141,27 @@ CodeGenerator::emitInstanceOf(LInstructi
     masm.mov(Imm32(0), output);
 
     // Walk the prototype chain
     masm.bind(&loopPrototypeChain);
     masm.loadPtr(Address(lhsTmp, JSObject::offsetOfType()), lhsTmp);
     masm.loadPtr(Address(lhsTmp, offsetof(types::TypeObject, proto)), lhsTmp);
 
     // Bail out if we hit a lazy proto
-    masm.branch32(Assembler::Equal, lhsTmp, Imm32(1), call->entry());
+    if (lhsIsValue) {
+        masm.branch32(Assembler::Equal, lhsTmp, Imm32(1), call->entry());
+    } else {
+        // If the input LHS is raw object pointer, it must be boxed before
+        // calling into js::HasInstance.
+        Label dontCallHasInstance;
+        masm.branch32(Assembler::NotEqual, lhsTmp, Imm32(1), &dontCallHasInstance);
+        masm.boxNonDouble(JSVAL_TYPE_OBJECT, ToRegister(ins->getOperand(0)), ToTempValue(ins, 0));
+        masm.jump(call->entry());
+        masm.bind(&dontCallHasInstance);
+    }
 
     masm.testPtr(lhsTmp, lhsTmp);
     masm.j(Assembler::Zero, &done);
 
     // Check lhs is equal to rhsShape
     masm.cmpPtr(lhsTmp, rhsTmp);
     masm.j(Assembler::NotEqual, &loopPrototypeChain);
 
--- a/js/src/ion/arm/CodeGenerator-arm.cpp
+++ b/js/src/ion/arm/CodeGenerator-arm.cpp
@@ -1024,16 +1024,24 @@ CodeGeneratorARM::ToValue(LInstruction *
 ValueOperand
 CodeGeneratorARM::ToOutValue(LInstruction *ins)
 {
     Register typeReg = ToRegister(ins->getDef(TYPE_INDEX));
     Register payloadReg = ToRegister(ins->getDef(PAYLOAD_INDEX));
     return ValueOperand(typeReg, payloadReg);
 }
 
+ValueOperand
+CodeGeneratorARM::ToTempValue(LInstruction *ins, size_t pos)
+{
+    Register typeReg = ToRegister(ins->getTemp(pos + TYPE_INDEX));
+    Register payloadReg = ToRegister(ins->getTemp(pos + PAYLOAD_INDEX));
+    return ValueOperand(typeReg, payloadReg);
+}
+
 bool
 CodeGeneratorARM::visitValue(LValue *value)
 {
     const ValueOperand out = ToOutValue(value);
 
     masm.moveValue(value->value(), out);
     return true;
 }
--- a/js/src/ion/arm/CodeGenerator-arm.h
+++ b/js/src/ion/arm/CodeGenerator-arm.h
@@ -105,16 +105,17 @@ class CodeGeneratorARM : public CodeGene
     virtual bool visitTruncateDToInt32(LTruncateDToInt32 *ins);
 
     // Out of line visitors.
     bool visitOutOfLineBailout(OutOfLineBailout *ool);
 
   protected:
     ValueOperand ToValue(LInstruction *ins, size_t pos);
     ValueOperand ToOutValue(LInstruction *ins);
+    ValueOperand ToTempValue(LInstruction *ins, size_t pos);
 
     // Functions for LTestVAndBranch.
     Register splitTagForTest(const ValueOperand &value);
 
     void storeElementTyped(const LAllocation *value, MIRType valueType, MIRType elementType,
                            const Register &elements, const LAllocation *index);
 
   protected:
--- a/js/src/ion/arm/MacroAssembler-arm.cpp
+++ b/js/src/ion/arm/MacroAssembler-arm.cpp
@@ -2223,16 +2223,22 @@ MacroAssemblerARMCompat::unboxPrivate(co
 }
 
 void
 MacroAssemblerARMCompat::boxDouble(const FloatRegister &src, const ValueOperand &dest)
 {
     as_vxfer(dest.payloadReg(), dest.typeReg(), VFPRegister(src), FloatToCore);
 }
 
+void
+MacroAssemblerARMCompat::boxNonDouble(JSValueType type, const Register &src, const ValueOperand &dest) {
+    if (src != dest.payloadReg())
+        ma_mov(src, dest.payloadReg());
+    ma_mov(ImmType(type), dest.typeReg());
+}
 
 void
 MacroAssemblerARMCompat::boolValueToDouble(const ValueOperand &operand, const FloatRegister &dest)
 {
     VFPRegister d = VFPRegister(dest);
     ma_vimm(1.0, dest);
     ma_cmp(operand.payloadReg(), Imm32(0));
     // If the source is 0, then subtract the dest from itself, producing 0.
--- a/js/src/ion/arm/MacroAssembler-arm.h
+++ b/js/src/ion/arm/MacroAssembler-arm.h
@@ -546,16 +546,17 @@ class MacroAssemblerARMCompat : public M
     void unboxBoolean(const ValueOperand &operand, const Register &dest);
     void unboxBoolean(const Address &src, const Register &dest);
     void unboxDouble(const ValueOperand &operand, const FloatRegister &dest);
     void unboxValue(const ValueOperand &src, AnyRegister dest);
     void unboxPrivate(const ValueOperand &src, Register dest);
 
     // boxing code
     void boxDouble(const FloatRegister &src, const ValueOperand &dest);
+    void boxNonDouble(JSValueType type, const Register &src, const ValueOperand &dest);
 
     // Extended unboxing API. If the payload is already in a register, returns
     // that register. Otherwise, provides a move to the given scratch register,
     // and returns that.
     Register extractObject(const Address &address, Register scratch);
     Register extractObject(const ValueOperand &value, Register scratch) {
         return value.payloadReg();
     }
--- a/js/src/ion/x64/CodeGenerator-x64.cpp
+++ b/js/src/ion/x64/CodeGenerator-x64.cpp
@@ -28,16 +28,22 @@ CodeGeneratorX64::ToValue(LInstruction *
 }
 
 ValueOperand
 CodeGeneratorX64::ToOutValue(LInstruction *ins)
 {
     return ValueOperand(ToRegister(ins->getDef(0)));
 }
 
+ValueOperand
+CodeGeneratorX64::ToTempValue(LInstruction *ins, size_t pos)
+{
+    return ValueOperand(ToRegister(ins->getTemp(pos)));
+}
+
 bool
 CodeGeneratorX64::visitDouble(LDouble *ins)
 {
     const LDefinition *out = ins->output();
     masm.loadConstantDouble(ins->getDouble(), ToFloatRegister(out));
     return true;
 }
 
--- a/js/src/ion/x64/CodeGenerator-x64.h
+++ b/js/src/ion/x64/CodeGenerator-x64.h
@@ -18,16 +18,17 @@ class CodeGeneratorX64 : public CodeGene
 {
     CodeGeneratorX64 *thisFromCtor() {
         return this;
     }
 
   protected:
     ValueOperand ToValue(LInstruction *ins, size_t pos);
     ValueOperand ToOutValue(LInstruction *ins);
+    ValueOperand ToTempValue(LInstruction *ins, size_t pos);
 
 
     void loadUnboxedValue(Operand source, MIRType type, const LDefinition *dest);
     void storeUnboxedValue(const LAllocation *value, MIRType valueType,
                            Operand dest, MIRType slotType);
 
     void storeElementTyped(const LAllocation *value, MIRType valueType, MIRType elementType,
                            const Register &elements, const LAllocation *index);
--- a/js/src/ion/x64/MacroAssembler-x64.h
+++ b/js/src/ion/x64/MacroAssembler-x64.h
@@ -642,16 +642,20 @@ class MacroAssemblerX64 : public MacroAs
         moveValue(v, ScratchReg);
         cmpq(value.valueReg(), ScratchReg);
         j(cond, label);
     }
 
     void boxDouble(const FloatRegister &src, const ValueOperand &dest) {
         movqsd(src, dest.valueReg());
     }
+    void boxNonDouble(JSValueType type, const Register &src, const ValueOperand &dest) {
+        JS_ASSERT(src != dest.valueReg());
+        boxValue(type, src, dest.valueReg());
+    }
 
     // Note that the |dest| register here may be ScratchReg, so we shouldn't
     // use it.
     void unboxInt32(const ValueOperand &src, const Register &dest) {
         movl(Operand(src.valueReg()), dest);
     }
     void unboxInt32(const Operand &src, const Register &dest) {
         movl(src, dest);
--- a/js/src/ion/x86/CodeGenerator-x86.cpp
+++ b/js/src/ion/x86/CodeGenerator-x86.cpp
@@ -65,16 +65,24 @@ CodeGeneratorX86::ToValue(LInstruction *
 ValueOperand
 CodeGeneratorX86::ToOutValue(LInstruction *ins)
 {
     Register typeReg = ToRegister(ins->getDef(TYPE_INDEX));
     Register payloadReg = ToRegister(ins->getDef(PAYLOAD_INDEX));
     return ValueOperand(typeReg, payloadReg);
 }
 
+ValueOperand
+CodeGeneratorX86::ToTempValue(LInstruction *ins, size_t pos)
+{
+    Register typeReg = ToRegister(ins->getTemp(pos + TYPE_INDEX));
+    Register payloadReg = ToRegister(ins->getTemp(pos + PAYLOAD_INDEX));
+    return ValueOperand(typeReg, payloadReg);
+}
+
 bool
 CodeGeneratorX86::visitValue(LValue *value)
 {
     const ValueOperand out = ToOutValue(value);
     masm.moveValue(value->value(), out);
     return true;
 }
 
--- a/js/src/ion/x86/CodeGenerator-x86.h
+++ b/js/src/ion/x86/CodeGenerator-x86.h
@@ -38,16 +38,17 @@ class CodeGeneratorX86 : public CodeGene
 
     CodeGeneratorX86 *thisFromCtor() {
         return this;
     }
 
   protected:
     ValueOperand ToValue(LInstruction *ins, size_t pos);
     ValueOperand ToOutValue(LInstruction *ins);
+    ValueOperand ToTempValue(LInstruction *ins, size_t pos);
 
     void storeElementTyped(const LAllocation *value, MIRType valueType, MIRType elementType,
                            const Register &elements, const LAllocation *index);
 
   protected:
     void linkAbsoluteLabels();
 
   public:
--- a/js/src/ion/x86/MacroAssembler-x86.h
+++ b/js/src/ion/x86/MacroAssembler-x86.h
@@ -528,16 +528,21 @@ class MacroAssemblerX86 : public MacroAs
     }
 
     // Note: this function clobbers the source register.
     void boxDouble(const FloatRegister &src, const ValueOperand &dest) {
         movd(src, dest.payloadReg());
         psrldq(Imm32(4), src);
         movd(src, dest.typeReg());
     }
+    void boxNonDouble(JSValueType type, const Register &src, const ValueOperand &dest) {
+        if (src != dest.payloadReg())
+            movl(src, dest.payloadReg());
+        movl(ImmType(type), dest.typeReg());
+    }
     void unboxInt32(const ValueOperand &src, const Register &dest) {
         movl(src.payloadReg(), dest);
     }
     void unboxInt32(const Address &src, const Register &dest) {
         movl(payloadOf(src), dest);
     }
     void unboxBoolean(const ValueOperand &src, const Register &dest) {
         movl(src.payloadReg(), dest);