Bug 625701: Sensible fixed width branching. (r=cdleary)
authorJacob Bramley <Jacob.Bramley@arm.com>
Fri, 14 Jan 2011 09:00:56 -0800
changeset 60602 fbcba31f6380f931db2f9dbe4b514161ada03936
parent 60601 fb2192c7b8c272818d7fbd9433538e6395bd876d
child 60603 2fdb8bbd034141d326e8c51199d8a393a3ac3cd9
push idunknown
push userunknown
push dateunknown
reviewerscdleary
bugs625701
milestone2.0b10pre
Bug 625701: Sensible fixed width branching. (r=cdleary)
js/src/assembler/assembler/MacroAssemblerARM.h
js/src/assembler/assembler/MacroAssemblerX86Common.h
js/src/methodjit/PolyIC.cpp
--- a/js/src/assembler/assembler/MacroAssemblerARM.h
+++ b/js/src/assembler/assembler/MacroAssemblerARM.h
@@ -491,22 +491,27 @@ public:
         if (right.m_isPointer) {
             m_assembler.ldr_un_imm(ARMRegisters::S0, right.m_value);
             m_assembler.cmp_r(left, ARMRegisters::S0);
         } else
             m_assembler.cmp_r(left, m_assembler.getImm(right.m_value, ARMRegisters::S0));
         return Jump(m_assembler.jmp(ARMCondition(cond), useConstantPool));
     }
 
-    Jump branch32_force32(Condition cond, RegisterID left, Imm32 right, int useConstantPool = 0)
+    // Like branch32, but emit a consistently-structured sequence such that the
+    // number of instructions emitted is constant, regardless of the argument
+    // values. For ARM, this is identical to branch32WithPatch, except that it
+    // does not generate a DataLabel32.
+    Jump branch32FixedLength(Condition cond, RegisterID left, Imm32 right)
     {
-        return branch32(cond, left, right, useConstantPool);
+        m_assembler.ldr_un_imm(ARMRegisters::S1, right.m_value);
+        return branch32(cond, left, ARMRegisters::S1, true);
     }
 
-    // As branch32, but allow the value ('right') to be patched.
+    // As branch32_force32, but allow the value ('right') to be patched.
     Jump branch32WithPatch(Condition cond, RegisterID left, Imm32 right, DataLabel32 &dataLabel)
     {
         ASSERT(left != ARMRegisters::S1);
         dataLabel = moveWithPatch(right, ARMRegisters::S1);
         return branch32(cond, left, ARMRegisters::S1, true);
     }
 
     Jump branch32WithPatch(Condition cond, Address left, Imm32 right, DataLabel32 &dataLabel)
--- a/js/src/assembler/assembler/MacroAssemblerX86Common.h
+++ b/js/src/assembler/assembler/MacroAssemblerX86Common.h
@@ -851,28 +851,29 @@ public:
         if (((cond == Equal) || (cond == NotEqual)) && !right.m_value)
             m_assembler.testl_rr(left, left);
         else
             m_assembler.cmpl_ir(right.m_value, left);
         return Jump(m_assembler.jCC(x86Condition(cond)));
     }
     
     // Branch based on a 32-bit comparison, forcing the size of the
-    // immediate operand to 32 bits in the native code stream.
-    Jump branch32_force32(Condition cond, RegisterID left, Imm32 right)
+    // immediate operand to 32 bits in the native code stream to ensure that
+    // the length of code emitted by this instruction is consistent.
+    Jump branch32FixedLength(Condition cond, RegisterID left, Imm32 right)
     {
         m_assembler.cmpl_ir_force32(right.m_value, left);
         return Jump(m_assembler.jCC(x86Condition(cond)));
     }
 
     // Branch and record a label after the comparison.
     Jump branch32WithPatch(Condition cond, RegisterID left, Imm32 right, DataLabel32 &dataLabel)
     {
         // Always use cmpl, since the value is to be patched.
-        m_assembler.cmpl_ir(right.m_value, left);
+        m_assembler.cmpl_ir_force32(right.m_value, left);
         dataLabel = DataLabel32(this);
         return Jump(m_assembler.jCC(x86Condition(cond)));
     }
 
     Jump branch32WithPatch(Condition cond, Address left, Imm32 right, DataLabel32 &dataLabel)
     {
         m_assembler.cmpl_im_force32(right.m_value, left.offset, left.base);
         dataLabel = DataLabel32(this);
--- a/js/src/methodjit/PolyIC.cpp
+++ b/js/src/methodjit/PolyIC.cpp
@@ -282,18 +282,18 @@ class SetPropCompiler : public PICStubCo
 
         // Shape guard.
         if (pic.shapeNeedsRemat()) {
             masm.loadShape(pic.objReg, pic.shapeReg);
             pic.shapeRegHasBaseShape = true;
         }
 
         Label start = masm.label();
-        Jump shapeGuard = masm.branch32_force32(Assembler::NotEqual, pic.shapeReg,
-                                                Imm32(initialShape));
+        Jump shapeGuard = masm.branch32FixedLength(Assembler::NotEqual, pic.shapeReg,
+                                                   Imm32(initialShape));
 
         Label stubShapeJumpLabel = masm.label();
 
         pic.setPropLabels().setStubShapeJump(masm, start, stubShapeJumpLabel);
 
         JS_ASSERT_IF(!shape->hasDefaultSetter(), obj->getClass() == &js_CallClass);
 
         MaybeJump skipOver;
@@ -1050,18 +1050,18 @@ class GetPropCompiler : public PICStubCo
 #endif
         } else {
             if (pic.shapeNeedsRemat()) {
                 masm.loadShape(pic.objReg, pic.shapeReg);
                 pic.shapeRegHasBaseShape = true;
             }
 
             start = masm.label();
-            shapeGuardJump = masm.branch32_force32(Assembler::NotEqual, pic.shapeReg,
-                                                   Imm32(obj->shape()));
+            shapeGuardJump = masm.branch32FixedLength(Assembler::NotEqual, pic.shapeReg,
+                                                      Imm32(obj->shape()));
         }
         Label stubShapeJumpLabel = masm.label();
 
         if (!shapeMismatches.append(shapeGuardJump))
             return error();
 
         RegisterID holderReg = pic.objReg;
         if (obj != holder) {