[JAEGER] Better x64 Value loading. b=586240, r=dvander.
authorSean Stangl <sstangl@mozilla.com>
Wed, 11 Aug 2010 13:45:57 -0700
changeset 53386 7dd58614272f8d60eca8f57a96a683e94ee85a65
parent 53385 a7590ac9f0329a7149950e511d1023434fc6c2c5
child 53387 f7cf2b7b79613a6f27033c130d637a1cfb3ea8a3
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs586240
milestone2.0b4pre
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
[JAEGER] Better x64 Value loading. b=586240, r=dvander.
js/src/methodjit/Compiler.cpp
js/src/methodjit/FrameState.cpp
js/src/methodjit/PunboxAssembler.h
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -2130,22 +2130,18 @@ mjit::Compiler::jsop_getprop(JSAtom *ato
 
 #if defined JS_NUNBOX32
     masm.loadTypeTag(slot, shapeReg);
     DBGLABEL(dbgTypeLoad);
 
     masm.loadPayload(slot, objReg);
     DBGLABEL(dbgDataLoad);
 #elif defined JS_PUNBOX64
-    masm.loadValue(slot, shapeReg);
-    Label inlineValueLoadLabel = masm.label();
-
-    masm.move(shapeReg, objReg);
-    masm.convertValueToPayload(objReg);
-    masm.convertValueToType(shapeReg);
+    Label inlineValueLoadLabel =
+        masm.loadValueAsComponents(slot, shapeReg, objReg);
 #endif
     pic.storeBack = masm.label();
 
 
     /* Assert correctness of hardcoded offsets. */
 #if defined JS_NUNBOX32
     JS_ASSERT(masm.differenceBetween(pic.storeBack, dbgDslotsLoad) == GETPROP_DSLOTS_LOAD);
     JS_ASSERT(masm.differenceBetween(pic.storeBack, dbgTypeLoad) == GETPROP_TYPE_LOAD);
@@ -2223,22 +2219,18 @@ mjit::Compiler::jsop_getelem_pic(FrameEn
     /* Copy the slot value to the expression stack. */
     Address slot(objReg, 1 << 24);
 #if defined JS_NUNBOX32
     masm.loadTypeTag(slot, shapeReg);
     DBGLABEL(dbgTypeLoad);
     masm.loadPayload(slot, objReg);
     DBGLABEL(dbgDataLoad);
 #elif defined JS_PUNBOX64
-    masm.loadValue(slot, shapeReg);
-    Label inlineValueOffsetLabel = masm.label();
-
-    masm.move(shapeReg, objReg);
-    masm.convertValueToType(shapeReg);
-    masm.convertValueToPayload(objReg);
+    Label inlineValueOffsetLabel =
+        masm.loadValueAsComponents(slot, shapeReg, objReg);
 #endif
     pic.storeBack = masm.label();
 
     pic.objReg = objReg;
     pic.idReg = idReg;
 
 #if defined JS_NUNBOX32
     JS_ASSERT(masm.differenceBetween(pic.storeBack, dbgDslotsLoad) == GETELEM_DSLOTS_LOAD);
@@ -2360,22 +2352,18 @@ mjit::Compiler::jsop_callprop_generic(JS
 
 #if defined JS_NUNBOX32
     masm.loadTypeTag(slot, shapeReg);
     DBGLABEL(dbgTypeLoad);
 
     masm.loadPayload(slot, objReg);
     DBGLABEL(dbgDataLoad);
 #elif defined JS_PUNBOX64
-    masm.loadValue(slot, shapeReg);
-    Label inlineValueLoadLabel = masm.label();
-
-    masm.move(shapeReg, objReg);
-    masm.convertValueToPayload(objReg);
-    masm.convertValueToType(shapeReg);
+    Label inlineValueLoadLabel =
+        masm.loadValueAsComponents(slot, shapeReg, objReg);
 #endif
     pic.storeBack = masm.label();
 
     /* Assert correctness of hardcoded offsets. */
     JS_ASSERT(masm.differenceBetween(pic.fastPathStart, dbgInlineTypeGuard) == GETPROP_INLINE_TYPE_GUARD);
 #if defined JS_NUNBOX32
     JS_ASSERT(masm.differenceBetween(pic.storeBack, dbgDslotsLoad) == GETPROP_DSLOTS_LOAD);
     JS_ASSERT(masm.differenceBetween(pic.storeBack, dbgTypeLoad) == GETPROP_TYPE_LOAD);
@@ -2519,22 +2507,18 @@ mjit::Compiler::jsop_callprop_obj(JSAtom
 
 #if defined JS_NUNBOX32
     masm.loadTypeTag(slot, shapeReg);
     DBGLABEL(dbgTypeLoad);
 
     masm.loadPayload(slot, objReg);
     DBGLABEL(dbgDataLoad);
 #elif defined JS_PUNBOX64
-    masm.loadValue(slot, shapeReg);
-    Label inlineValueLoadLabel = masm.label();
-
-    masm.move(shapeReg, objReg);
-    masm.convertValueToPayload(objReg);
-    masm.convertValueToType(shapeReg);
+    Label inlineValueLoadLabel =
+        masm.loadValueAsComponents(slot, shapeReg, objReg);
 #endif
 
     pic.storeBack = masm.label();
     pic.objReg = objReg;
 
     /*
      * 1) Dup the |this| object.
      * 2) Push the property value onto the stack.
@@ -3308,22 +3292,19 @@ mjit::Compiler::jsop_getgname(uint32 ind
     /* After dreg is loaded, it's safe to clobber objReg. */
     RegisterID treg = objReg;
 
     mic.load = masm.label();
 # if defined JS_NUNBOX32
     masm.loadPayload(address, dreg);
     masm.loadTypeTag(address, treg);
 # elif defined JS_PUNBOX64
-    masm.loadValue(address, treg);
-    mic.patchValueOffset = masm.differenceBetween(mic.load, masm.label());
-
-    masm.move(treg, dreg);
-    masm.convertValueToPayload(dreg);
-    masm.convertValueToType(treg);
+    Label inlineValueLoadLabel =
+        masm.loadValueAsComponents(address, treg, dreg);
+    mic.patchValueOffset = masm.differenceBetween(mic.load, inlineValueLoadLabel);
 # endif
 
     frame.pushRegs(treg, dreg);
 
     stubcc.rejoin(Changes(1));
     mics.append(mic);
 
 #else
--- a/js/src/methodjit/FrameState.cpp
+++ b/js/src/methodjit/FrameState.cpp
@@ -213,30 +213,35 @@ FrameState::storeTo(FrameEntry *fe, Addr
     /* Cannot clobber the address's register. */
     JS_ASSERT(!freeRegs.hasReg(address.base));
 
     if (fe->data.inRegister()) {
         masm.storePayload(fe->data.reg(), address);
     } else {
         JS_ASSERT(fe->data.inMemory());
         RegisterID reg = popped ? allocReg() : allocReg(fe, RematInfo::DATA);
+
+        JS_ASSERT(addressOf(fe).base != address.base ||
+                  addressOf(fe).offset != address.offset);
         masm.loadPayload(addressOf(fe), reg);
         masm.storePayload(reg, address);
         if (popped)
             freeReg(reg);
         else
             fe->data.setRegister(reg);
     }
 
     if (fe->isTypeKnown()) {
         masm.storeTypeTag(ImmType(fe->getKnownType()), address);
     } else if (fe->type.inRegister()) {
         masm.storeTypeTag(fe->type.reg(), address);
     } else {
         JS_ASSERT(fe->type.inMemory());
+        JS_ASSERT(addressOf(fe).base != address.base ||
+                  addressOf(fe).offset != address.offset);
         RegisterID reg = popped ? allocReg() : allocReg(fe, RematInfo::TYPE);
         masm.loadTypeTag(addressOf(fe), reg);
         masm.storeTypeTag(reg, address);
         if (popped)
             freeReg(reg);
         else
             fe->type.setRegister(reg);
     }
@@ -477,20 +482,27 @@ FrameState::merge(Assembler &masm, Chang
 
         /* Copies do not have registers. */
         if (fe->isCopy()) {
             JS_ASSERT(!fe->data.inRegister());
             JS_ASSERT(!fe->type.inRegister());
             continue;
         }
 
-        if (fe->data.inRegister())
-            masm.loadPayload(addressOf(fe), fe->data.reg());
-        if (fe->type.inRegister())
-            masm.loadTypeTag(addressOf(fe), fe->type.reg());
+#if defined JS_PUNBOX64
+        if (fe->data.inRegister() && fe->type.inRegister()) {
+            masm.loadValueAsComponents(addressOf(fe), fe->type.reg(), fe->data.reg());
+        } else
+#endif
+        {
+            if (fe->data.inRegister())
+                masm.loadPayload(addressOf(fe), fe->data.reg());
+            if (fe->type.inRegister())
+                masm.loadTypeTag(addressOf(fe), fe->type.reg());
+        }
     }
 }
 
 JSC::MacroAssembler::RegisterID
 FrameState::copyDataIntoReg(FrameEntry *fe)
 {
     return copyDataIntoReg(this->masm, fe);
 }
--- a/js/src/methodjit/PunboxAssembler.h
+++ b/js/src/methodjit/PunboxAssembler.h
@@ -96,18 +96,17 @@ class Assembler : public BaseAssembler
     void loadSlot(RegisterID obj, RegisterID clobber, uint32 slot, RegisterID type, RegisterID data) {
         JS_ASSERT(type != data);
         Address address(obj, offsetof(JSObject, fslots) + slot * sizeof(Value));
         if (slot >= JS_INITIAL_NSLOTS) {
             loadPtr(Address(obj, offsetof(JSObject, dslots)), clobber);
             address = Address(clobber, (slot - JS_INITIAL_NSLOTS) * sizeof(Value));
         }
         
-        loadValueThenPayload(address, type, data);
-        convertValueToType(type);
+        loadValueAsComponents(address, type, data);
     }
 
     void loadValue(Address address, RegisterID dst) {
         loadPtr(address, dst);
     }
 
     void loadValue(BaseIndex address, RegisterID dst) {
         loadPtr(address, dst);
@@ -116,102 +115,101 @@ class Assembler : public BaseAssembler
     void convertValueToType(RegisterID val) {
         andPtr(Imm64(JSVAL_TAG_MASK), val);
     }
 
     void convertValueToPayload(RegisterID val) {
         andPtr(Imm64(JSVAL_PAYLOAD_MASK), val);
     }
 
-    void loadValueThenType(Address address, RegisterID val, RegisterID type) {
-        loadValue(valueOf(address), val);
-        if (val != type)
-            move(val, type);
-        convertValueToType(type);
-    }
-    
-    void loadValueThenType(BaseIndex address, RegisterID val, RegisterID type) {
-        loadValue(valueOf(address), val);
-        if (val != type)
-            move(val, type);
+    /* Returns a label after the one Value load. */
+    Label loadValueAsComponents(Address address, RegisterID type, RegisterID payload) {
+        /*
+         * Loading into the ValueRegister is better than loading directly into type:
+         * with this code, there are no interdependencies between type and payload,
+         * allowing the instructions to be better pipelined. This is X64-specific.
+         */
+        loadValue(address, Registers::ValueReg);
+        Label l = label();
+        
+        move(Registers::ValueReg, type);
+        move(Registers::ValueReg, payload);
+        move(Imm64(JSVAL_PAYLOAD_MASK), Registers::ValueReg);
+
+        /* Use JSC::scratchRegister to mask type bits. */
         convertValueToType(type);
-    }
 
-    void loadValueThenPayload(Address address, RegisterID val, RegisterID payload) {
-        loadValue(valueOf(address), val);
-        if (val != payload)
-            move(val, payload);
-        convertValueToPayload(payload);
-    }
+        /* Use ValueReg to mask payload bits. */
+        andPtr(Registers::ValueReg, payload);
 
-    void loadValueThenPayload(BaseIndex address, RegisterID val, RegisterID payload) {
-        loadValue(valueOf(address), val);
-        if (val != payload)
-            move(val, payload);
-        convertValueToPayload(payload);
+        return l;
     }
 
     void loadTypeTag(Address address, RegisterID reg) {
-        loadValueThenType(valueOf(address), reg, reg);
+        loadValue(address, reg);
+        convertValueToType(reg);
     }
 
     void loadTypeTag(BaseIndex address, RegisterID reg) {
-        loadValueThenType(valueOf(address), reg, reg);
+        loadValue(address, reg);
+        convertValueToType(reg);
     }
 
     void storeTypeTag(ImmShiftedTag imm, Address address) {
-        loadValue(valueOf(address), Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
         convertValueToPayload(Registers::ValueReg);
         orPtr(imm, Registers::ValueReg);
         storePtr(Registers::ValueReg, valueOf(address));
     }
 
     void storeTypeTag(ImmShiftedTag imm, BaseIndex address) {
-        loadValue(valueOf(address), Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
         convertValueToPayload(Registers::ValueReg);
         orPtr(imm, Registers::ValueReg);
         storePtr(Registers::ValueReg, valueOf(address));
     }
 
     void storeTypeTag(RegisterID reg, Address address) {
         /* The type tag must be stored in shifted format. */
-        loadValue(valueOf(address), Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
         convertValueToPayload(Registers::ValueReg);
         orPtr(reg, Registers::ValueReg);
         storePtr(Registers::ValueReg, valueOf(address));
 
     }
 
     void storeTypeTag(RegisterID reg, BaseIndex address) {
         /* The type tag must be stored in shifted format. */
-        loadValue(valueOf(address), Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
         convertValueToPayload(Registers::ValueReg);
         orPtr(reg, Registers::ValueReg);
         storePtr(Registers::ValueReg, valueOf(address));
     }
 
     void loadPayload(Address address, RegisterID reg) {
-        loadValueThenPayload(address, reg, reg);
+        loadValue(address, reg);
+        convertValueToPayload(reg);
     }
 
     void loadPayload(BaseIndex address, RegisterID reg) {
-        loadValueThenPayload(address, reg, reg);
+        loadValue(address, reg);
+        convertValueToPayload(reg);
     }
 
     void storePayload(RegisterID reg, Address address) {
         /* Not for doubles. */
-        loadValue(valueOf(address), Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
         convertValueToType(Registers::ValueReg);
         orPtr(reg, Registers::ValueReg);
         storePtr(Registers::ValueReg, valueOf(address));
     }
 
     void storePayload(RegisterID reg, BaseIndex address) {
         /* Not for doubles. */
-        loadValue(valueOf(address), Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
         convertValueToType(Registers::ValueReg);
         orPtr(reg, Registers::ValueReg);
         storePtr(Registers::ValueReg, valueOf(address));
     }
     
     void storePayload(Imm64 imm, Address address) {
         /* Not for doubles. */
         storePtr(imm, valueOf(address));
@@ -250,57 +248,62 @@ class Assembler : public BaseAssembler
         lshiftPtr(Imm32(1), to);
     }
 
     Jump testNull(Assembler::Condition cond, RegisterID reg) {
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_NULL));
     }
 
     Jump testNull(Assembler::Condition cond, Address address) {
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_NULL));
     }
 
     Jump testInt32(Assembler::Condition cond, RegisterID reg) {
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_INT32));
     }
 
     Jump testInt32(Assembler::Condition cond, Address address) {
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_INT32));
     }
 
     Jump testNumber(Assembler::Condition cond, RegisterID reg) {
         cond = (cond == Assembler::Equal) ? Assembler::BelowOrEqual : Assembler::Above;
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_INT32));
     }
 
     Jump testNumber(Assembler::Condition cond, Address address) {
         cond = (cond == Assembler::Equal) ? Assembler::BelowOrEqual : Assembler::Above;
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_INT32));
     }
 
     Jump testPrimitive(Assembler::Condition cond, RegisterID reg) {
         cond = (cond == Assembler::NotEqual) ? Assembler::AboveOrEqual : Assembler::Below;
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_OBJECT));
     }
 
     Jump testPrimitive(Assembler::Condition cond, Address address) {
         cond = (cond == Assembler::NotEqual) ? Assembler::AboveOrEqual : Assembler::Below;
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_OBJECT));
     }
 
     Jump testObject(Assembler::Condition cond, RegisterID reg) {
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_OBJECT));
     }
 
     Jump testObject(Assembler::Condition cond, Address address) {
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_OBJECT));
     }
 
     Jump testDouble(Assembler::Condition cond, RegisterID reg) {
         Assembler::Condition opcond;
         if (cond == Assembler::Equal)
             opcond = Assembler::Below;
         else
@@ -309,35 +312,38 @@ class Assembler : public BaseAssembler
     }
 
     Jump testDouble(Assembler::Condition cond, Address address) {
         Assembler::Condition opcond;
         if (cond == Assembler::Equal)
             opcond = Assembler::Below;
         else
             opcond = Assembler::AboveOrEqual;
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(opcond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_MAX_DOUBLE));
     }
 
     Jump testBoolean(Assembler::Condition cond, RegisterID reg) {
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_BOOLEAN));
     }
 
     Jump testBoolean(Assembler::Condition cond, Address address) {
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_BOOLEAN));
     }
 
     Jump testString(Assembler::Condition cond, RegisterID reg) {
         return branchPtr(cond, reg, ImmShiftedTag(JSVAL_SHIFTED_TAG_STRING));
     }
 
     Jump testString(Assembler::Condition cond, Address address) {
-        loadValueThenType(address, Registers::ValueReg, Registers::ValueReg);
+        loadValue(address, Registers::ValueReg);
+        convertValueToType(Registers::ValueReg);
         return branchPtr(cond, Registers::ValueReg, ImmShiftedTag(JSVAL_SHIFTED_TAG_BOOLEAN));
     }
 };
 
 } /* namespace js */
 } /* namespace mjit */
 
 #endif