Changeset 190649 in webkit

Timestamp:

Oct 6, 2015, 3:29:27 PM (10 years ago)

Author:

mark.lam@apple.com

Message:

Factoring out op_sub baseline code generation into JITSubGenerator.
​https://bugs.webkit.org/show_bug.cgi?id=149600

Reviewed by Geoffrey Garen.

We're going to factor out baseline code generation into snippet generators so
that we can later use them in the DFG and FTL to emit code for to perform the
JS operations where the operand types are predicted to be polymorphic.
We are starting in this patch with the implementation of op_sub.

What was done in this patch:

1. Created JITSubGenerator based on the baseline implementation of op_sub as expressed in compileBinaryArithOp() and compileBinaryArithOpSlowCase(). I did not attempt to do write a more optimal version of op_sub. I'll leave that to a later patch.

2. Convert the 32-bit op_sub baseline implementation to use the same JITSubGenerator which was based on the 64-bit implementation. The pre-existing 32-bit baseline op_sub had handling for more optimization cases. However, a benchmark run shows that simply going with the 64-bit version (foregoing those extra optimizations) did not change the performance.

Also, previously, the 32-bit version was able to move double results directly
into the result location on the stack directly. By using JITSubGenerator,
we now always move that result into a pair of GPRs before storing it into
the stack location.

3. Add some needed emitters to AssemblyHelpers that play nice with JSValueRegs.

• JavaScriptCore.xcodeproj/project.pbxproj:
• jit/AssemblyHelpers.h:

(JSC::AssemblyHelpers::boxDouble):
(JSC::AssemblyHelpers::unboxDouble):
(JSC::AssemblyHelpers::boxBooleanPayload):

• jit/JIT.h:

(JSC::JIT::linkDummySlowCase):

• jit/JITArithmetic.cpp:

(JSC::JIT::compileBinaryArithOp):
(JSC::JIT::compileBinaryArithOpSlowCase):
(JSC::JIT::emitSlow_op_div):
(JSC::JIT::emit_op_sub):
(JSC::JIT::emitSlow_op_sub):

• jit/JITArithmetic32_64.cpp:

(JSC::JIT::emitBinaryDoubleOp):
(JSC::JIT::emit_op_sub): Deleted.
(JSC::JIT::emitSub32Constant): Deleted.
(JSC::JIT::emitSlow_op_sub): Deleted.

• jit/JITInlines.h:

(JSC::JIT::linkSlowCaseIfNotJSCell):
(JSC::JIT::linkAllSlowCasesForBytecodeOffset):
(JSC::JIT::addSlowCase):
(JSC::JIT::emitLoad):
(JSC::JIT::emitGetVirtualRegister):
(JSC::JIT::emitPutVirtualRegister):

• jit/JITSubGenerator.h: Added.

(JSC::JITSubGenerator::JITSubGenerator):
(JSC::JITSubGenerator::generateFastPath):
(JSC::JITSubGenerator::slowPathJumpList):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• JavaScriptCore.xcodeproj/project.pbxproj (modified) (2 diffs)
• jit/AssemblyHelpers.h (modified) (2 diffs)
• jit/JIT.h (modified) (2 diffs)
• jit/JITArithmetic.cpp (modified) (7 diffs)
• jit/JITArithmetic32_64.cpp (modified) (4 diffs)
• jit/JITInlines.h (modified) (4 diffs)
• jit/JITSubGenerator.h (added)

trunk/Source/JavaScriptCore/ChangeLog ¶

@@ +1 @@
+2015-10-06  Mark Lam  <mark.lam@apple.com>
+
+        Factoring out op_sub baseline code generation into JITSubGenerator.
+        https://bugs.webkit.org/show_bug.cgi?id=149600
+
+        Reviewed by Geoffrey Garen.
+
+        We're going to factor out baseline code generation into snippet generators so
+        that we can later use them in the DFG and FTL to emit code for to perform the
+        JS operations where the operand types are predicted to be polymorphic.
+        We are starting in this patch with the implementation of op_sub.
+
+        What was done in this patch:
+        1. Created JITSubGenerator based on the baseline implementation of op_sub as
+           expressed in compileBinaryArithOp() and compileBinaryArithOpSlowCase().
+           I did not attempt to do write a more optimal version of op_sub.  I'll
+           leave that to a later patch.
+
+        2. Convert the 32-bit op_sub baseline implementation to use the same
+           JITSubGenerator which was based on the 64-bit implementation.  The
+           pre-existing 32-bit baseline op_sub had handling for more optimization cases.
+           However, a benchmark run shows that simply going with the 64-bit version
+           (foregoing those extra optimizations) did not change the performance.
+
+           Also, previously, the 32-bit version was able to move double results directly
+           into the result location on the stack directly.  By using JITSubGenerator,
+           we now always move that result into a pair of GPRs before storing it into
+           the stack location.
+
+        3. Add some needed emitters to AssemblyHelpers that play nice with JSValueRegs.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * jit/AssemblyHelpers.h:
+        (JSC::AssemblyHelpers::boxDouble):
+        (JSC::AssemblyHelpers::unboxDouble):
+        (JSC::AssemblyHelpers::boxBooleanPayload):
+        * jit/JIT.h:
+        (JSC::JIT::linkDummySlowCase):
+        * jit/JITArithmetic.cpp:
+        (JSC::JIT::compileBinaryArithOp):
+        (JSC::JIT::compileBinaryArithOpSlowCase):
+        (JSC::JIT::emitSlow_op_div):
+        (JSC::JIT::emit_op_sub):
+        (JSC::JIT::emitSlow_op_sub):
+        * jit/JITArithmetic32_64.cpp:
+        (JSC::JIT::emitBinaryDoubleOp):
+        (JSC::JIT::emit_op_sub): Deleted.
+        (JSC::JIT::emitSub32Constant): Deleted.
+        (JSC::JIT::emitSlow_op_sub): Deleted.
+        * jit/JITInlines.h:
+        (JSC::JIT::linkSlowCaseIfNotJSCell):
+        (JSC::JIT::linkAllSlowCasesForBytecodeOffset):
+        (JSC::JIT::addSlowCase):
+        (JSC::JIT::emitLoad):
+        (JSC::JIT::emitGetVirtualRegister):
+        (JSC::JIT::emitPutVirtualRegister):
+        * jit/JITSubGenerator.h: Added.
+        (JSC::JITSubGenerator::JITSubGenerator):
+        (JSC::JITSubGenerator::generateFastPath):
+        (JSC::JITSubGenerator::slowPathJumpList):
+
 2015-10-06  Daniel Bates  <dbates@webkit.org>
 

trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj ¶

@@ -3708 +3708 @@
                 FE7BA60E1A1A7CEC00F1F7B4 /* HeapVerifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HeapVerifier.h; sourceTree = "<group>"; };
                 FE90BB3A1B7CF64E006B3F03 /* VMInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = VMInlines.h; sourceTree = "<group>"; };
+                FE98B5B61BB9AE110073E7A6 /* JITSubGenerator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JITSubGenerator.h; sourceTree = "<group>"; };
                 FEA0861E182B7A0400F6D851 /* Breakpoint.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Breakpoint.h; sourceTree = "<group>"; };
                 FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DebuggerPrimitives.h; sourceTree = "<group>"; };
@@ -4190 +4191 @@
                                 FEF6835C174343CC00A32E25 /* JITStubsX86_64.h */,
                                 A7A4AE0C17973B4D005612B1 /* JITStubsX86Common.h */,
+                                FE98B5B61BB9AE110073E7A6 /* JITSubGenerator.h */,
                                 0F5EF91B16878F78003E5C25 /* JITThunks.cpp */,
                                 0F5EF91C16878F78003E5C25 /* JITThunks.h */,

trunk/Source/JavaScriptCore/jit/AssemblyHelpers.h ¶

@@ -965 +965 @@
         boxDouble(fpr, regs.gpr());
     }
-    
+    void unboxDouble(JSValueRegs regs, FPRReg destFPR, FPRReg)
+    {
+        unboxDouble(regs.payloadGPR(), destFPR);
+    }
+
     // Here are possible arrangements of source, target, scratch:
     // - source, target, scratch can all be separate registers.
@@ -1002 +1006 @@
     {
         boxDouble(fpr, regs.tagGPR(), regs.payloadGPR());
+    }
+    void unboxDouble(JSValueRegs regs, FPRReg fpr, FPRReg scratchFPR)
+    {
+        unboxDouble(regs.tagGPR(), regs.payloadGPR(), fpr, scratchFPR);
     }
 #endif

trunk/Source/JavaScriptCore/jit/JIT.h ¶

@@ -400 +400 @@
 
         enum FinalObjectMode { MayBeFinal, KnownNotFinal };
+
+        void emitGetVirtualRegister(int src, JSValueRegs dst);
+        void emitPutVirtualRegister(int dst, JSValueRegs src);
 
 #if USE(JSVALUE32_64)
@@ -709 +712 @@
         }
         void linkSlowCaseIfNotJSCell(Vector<SlowCaseEntry>::iterator&, int virtualRegisterIndex);
+        void linkAllSlowCasesForBytecodeOffset(Vector<SlowCaseEntry>& slowCases,
+            Vector<SlowCaseEntry>::iterator&, unsigned bytecodeOffset);
 
         MacroAssembler::Call appendCallWithExceptionCheck(const FunctionPtr&);

trunk/Source/JavaScriptCore/jit/JITArithmetic.cpp ¶

@@ -1 +1 @@
 /*
- * Copyright (C) 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -33 +33 @@
 #include "JITOperations.h"
 #include "JITStubs.h"
+#include "JITSubGenerator.h"
 #include "JSArray.h"
 #include "JSFunction.h"
@@ -669 +670 @@
     if (opcodeID == op_add)
         addSlowCase(branchAdd32(Overflow, regT1, regT0));
-    else if (opcodeID == op_sub)
-        addSlowCase(branchSub32(Overflow, regT1, regT0));
     else {
         ASSERT(opcodeID == op_mul);
@@ -722 +721 @@
     Label stubFunctionCall(this);
 
-    JITSlowPathCall slowPathCall(this, currentInstruction, opcodeID == op_add ? slow_path_add : opcodeID == op_sub ? slow_path_sub : slow_path_mul);
+    JITSlowPathCall slowPathCall(this, currentInstruction, opcodeID == op_add ? slow_path_add : slow_path_mul);
     slowPathCall.call();
     Jump end = jump();
@@ -768 +767 @@
     if (opcodeID == op_add)
         addDouble(fpRegT2, fpRegT1);
-    else if (opcodeID == op_sub)
-        subDouble(fpRegT2, fpRegT1);
     else if (opcodeID == op_mul)
         mulDouble(fpRegT2, fpRegT1);
@@ -962 +959 @@
 }
 
+#endif // USE(JSVALUE64)
+
 void JIT::emit_op_sub(Instruction* currentInstruction)
 {
@@ -969 +968 @@
     OperandTypes types = OperandTypes::fromInt(currentInstruction[4].u.operand);
 
-    compileBinaryArithOp(op_sub, result, op1, op2, types);
-    emitPutVirtualRegister(result);
+#if USE(JSVALUE64)
+    JSValueRegs leftRegs = JSValueRegs(regT0);
+    JSValueRegs rightRegs = JSValueRegs(regT1);
+    JSValueRegs resultRegs = leftRegs;
+    GPRReg scratchGPR = InvalidGPRReg;
+    FPRReg scratchFPR = InvalidFPRReg;
+#else
+    JSValueRegs leftRegs = JSValueRegs(regT1, regT0);
+    JSValueRegs rightRegs = JSValueRegs(regT3, regT2);
+    JSValueRegs resultRegs = leftRegs;
+    GPRReg scratchGPR = regT4;
+    FPRReg scratchFPR = fpRegT2;
+#endif
+
+    emitGetVirtualRegister(op1, leftRegs);
+    emitGetVirtualRegister(op2, rightRegs);
+
+    JITSubGenerator gen(resultRegs, leftRegs, rightRegs, types.first(), types.second(),
+        fpRegT0, fpRegT1, scratchGPR, scratchFPR);
+
+    gen.generateFastPath(*this);
+    emitPutVirtualRegister(result, resultRegs);
+
+    addSlowCase(gen.slowPathJumpList());
 }
 
 void JIT::emitSlow_op_sub(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
 {
-    int result = currentInstruction[1].u.operand;
-    int op1 = currentInstruction[2].u.operand;
-    int op2 = currentInstruction[3].u.operand;
-    OperandTypes types = OperandTypes::fromInt(currentInstruction[4].u.operand);
-
-    compileBinaryArithOpSlowCase(currentInstruction, op_sub, iter, result, op1, op2, types, false, false);
+    linkAllSlowCasesForBytecodeOffset(m_slowCases, iter, m_bytecodeOffset);
+
+    JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_sub);
+    slowPathCall.call();
 }
 
 /* ------------------------------ END: OP_ADD, OP_SUB, OP_MUL ------------------------------ */
 
-#endif // USE(JSVALUE64)
-
 } // namespace JSC
 

trunk/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp ¶

@@ -1 +1 @@
 /*
-* Copyright (C) 2008 Apple Inc. All rights reserved.
+* Copyright (C) 2008, 2015 Apple Inc. All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
@@ -595 +595 @@
 // Subtraction (-)
 
-void JIT::emit_op_sub(Instruction* currentInstruction)
-{
-    int dst = currentInstruction[1].u.operand;
-    int op1 = currentInstruction[2].u.operand;
-    int op2 = currentInstruction[3].u.operand;
-    OperandTypes types = OperandTypes::fromInt(currentInstruction[4].u.operand);
-
-    JumpList notInt32Op1;
-    JumpList notInt32Op2;
-
-    if (isOperandConstantInt(op2)) {
-        emitSub32Constant(dst, op1, getConstantOperand(op2).asInt32(), types.first());
-        return;
-    }
-
-    emitLoad2(op1, regT1, regT0, op2, regT3, regT2);
-    notInt32Op1.append(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
-    notInt32Op2.append(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)));
-
-    // Int32 case.
-    addSlowCase(branchSub32(Overflow, regT2, regT0));
-    emitStoreInt32(dst, regT0, (op1 == dst || op2 == dst));
-
-    if (!supportsFloatingPoint()) {
-        addSlowCase(notInt32Op1);
-        addSlowCase(notInt32Op2);
-        return;
-    }
-    Jump end = jump();
-
-    // Double case.
-    emitBinaryDoubleOp(op_sub, dst, op1, op2, types, notInt32Op1, notInt32Op2);
-    end.link(this);
-}
-
-void JIT::emitSub32Constant(int dst, int op, int32_t constant, ResultType opType)
-{
-    // Int32 case.
-    emitLoad(op, regT1, regT0);
-    Jump notInt32 = branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag));
-    addSlowCase(branchSub32(Overflow, regT0, Imm32(constant), regT2, regT3));   
-    emitStoreInt32(dst, regT2, (op == dst));
-
-    // Double case.
-    if (!supportsFloatingPoint()) {
-        addSlowCase(notInt32);
-        return;
-    }
-    Jump end = jump();
-
-    notInt32.link(this);
-    if (!opType.definitelyIsNumber())
-        addSlowCase(branch32(Above, regT1, TrustedImm32(JSValue::LowestTag)));
-    move(Imm32(constant), regT2);
-    convertInt32ToDouble(regT2, fpRegT0);
-    emitLoadDouble(op, fpRegT1);
-    subDouble(fpRegT0, fpRegT1);
-    emitStoreDouble(dst, fpRegT1);
-
-    end.link(this);
-}
-
-void JIT::emitSlow_op_sub(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
-{
-    int op2 = currentInstruction[3].u.operand;
-    OperandTypes types = OperandTypes::fromInt(currentInstruction[4].u.operand);
-
-    if (isOperandConstantInt(op2)) {
-        linkSlowCase(iter); // overflow check
-
-        if (!supportsFloatingPoint() || !types.first().definitelyIsNumber())
-            linkSlowCase(iter); // int32 or double check
-    } else {
-        linkSlowCase(iter); // overflow check
-
-        if (!supportsFloatingPoint()) {
-            linkSlowCase(iter); // int32 check
-            linkSlowCase(iter); // int32 check
-        } else {
-            if (!types.first().definitelyIsNumber())
-                linkSlowCase(iter); // double check
-
-            if (!types.second().definitelyIsNumber()) {
-                linkSlowCase(iter); // int32 check
-                linkSlowCase(iter); // double check
-            }
-        }
-    }
-
-    JITSlowPathCall slowPathCall(this, currentInstruction, slow_path_sub);
-    slowPathCall.call();
-}
-
 void JIT::emitBinaryDoubleOp(OpcodeID opcodeID, int dst, int op1, int op2, OperandTypes types, JumpList& notInt32Op1, JumpList& notInt32Op2, bool op1IsInRegisters, bool op2IsInRegisters)
 {
@@ -729 +636 @@
                 addDouble(fpRegT2, fpRegT0);
                 emitStoreDouble(dst, fpRegT0);
-                break;
-            case op_sub:
-                emitLoadDouble(op1, fpRegT1);
-                subDouble(fpRegT0, fpRegT1);
-                emitStoreDouble(dst, fpRegT1);
                 break;
             case op_div: {
@@ -829 +731 @@
                 emitLoadDouble(op2, fpRegT2);
                 addDouble(fpRegT2, fpRegT0);
-                emitStoreDouble(dst, fpRegT0);
-                break;
-            case op_sub:
-                emitLoadDouble(op2, fpRegT2);
-                subDouble(fpRegT2, fpRegT0);
                 emitStoreDouble(dst, fpRegT0);
                 break;

trunk/Source/JavaScriptCore/jit/JITInlines.h ¶

@@ -770 +770 @@
 }
 
+ALWAYS_INLINE void JIT::linkAllSlowCasesForBytecodeOffset(Vector<SlowCaseEntry>& slowCases, Vector<SlowCaseEntry>::iterator& iter, unsigned bytecodeOffset)
+{
+    while (iter != slowCases.end() && iter->to == bytecodeOffset) {
+        iter->from.link(this);
+        ++iter;
+    }
+}
+
 ALWAYS_INLINE void JIT::addSlowCase(Jump jump)
 {
@@ -999 +1007 @@
 }
 
+ALWAYS_INLINE void JIT::emitGetVirtualRegister(int src, JSValueRegs dst)
+{
+    emitLoad(src, dst.tagGPR(), dst.payloadGPR());
+}
+
+ALWAYS_INLINE void JIT::emitPutVirtualRegister(int dst, JSValueRegs from)
+{
+    emitStore(dst, from.tagGPR(), from.payloadGPR());
+}
+
 inline void JIT::emitLoad(int index, RegisterID tag, RegisterID payload, RegisterID base)
 {
@@ -1157 +1175 @@
 }
 
+ALWAYS_INLINE void JIT::emitGetVirtualRegister(int src, JSValueRegs dst)
+{
+    emitGetVirtualRegister(src, dst.payloadGPR());
+}
+
 ALWAYS_INLINE void JIT::emitGetVirtualRegister(VirtualRegister src, RegisterID dst)
 {
@@ -1186 +1209 @@
 {
     store64(from, Address(callFrameRegister, dst * sizeof(Register)));
+}
+
+ALWAYS_INLINE void JIT::emitPutVirtualRegister(int dst, JSValueRegs from)
+{
+    emitPutVirtualRegister(dst, from.payloadGPR());
 }