Changeset 160205 in webkit


Ignore:
Timestamp:
Dec 5, 2013 5:47:19 PM (10 years ago)
Author:
fpizlo@apple.com
Message:

FTL should use cvttsd2si directly for double-to-int32 conversions
​https://bugs.webkit.org/show_bug.cgi?id=125275

Source/JavaScriptCore:

Reviewed by Michael Saboff.

Wow. This was an ordeal. Using cvttsd2si was actually easy, but I learned, and
sometimes even fixed, some interesting things:

  • The llvm.x86.sse2.cvttsd2si intrinsic can actually result in LLVM emitting a vcvttsd2si. I guess the intrinsic doesn't actually imply the instruction.


  • That whole thing about branchTruncateDoubleToUint32? Yeah we don't need that. It's better to use branchTruncateDoubleToInt32 instead. It has the right semantics for all of its callers (err, its one-and-only caller), and it's more likely to take fast path. This patch kills branchTruncateDoubleToUint32.


  • "a[i] = v; v = a[i]". Does this change v? OK, assume that 'a[i]' is a pure-ish operation - like an array access with 'i' being an integer index and we're not having a bad time. Now does this change v? CSE assumes that it doesn't. That's wrong. If 'a' is a typed array - the most sensible and pure kind of array - then this can be a truncating cast. For example 'v' could be a double and 'a' could be an integer array.


  • "v1 = a[i]; v2 = a[i]". Is v1 === v2 assuming that 'a[i]' is pure-ish? The answer is no. You could have a different arrayMode in each access. I know this sounds weird, but with concurrent JIT that might happen.


This patch adds tests for all of this stuff, except for the first issue (it's weird
but probably doesn't matter) and the last issue (it's too much of a freakshow).

  • assembler/MacroAssemblerARM64.h:
  • assembler/MacroAssemblerARMv7.h:
  • assembler/MacroAssemblerX86Common.h:
  • dfg/DFGCSEPhase.cpp:

(JSC::DFG::CSEPhase::getByValLoadElimination):
(JSC::DFG::CSEPhase::performNodeCSE):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):

  • ftl/FTLAbbreviations.h:

(JSC::FTL::vectorType):
(JSC::FTL::getUndef):
(JSC::FTL::buildInsertElement):

  • ftl/FTLIntrinsicRepository.h:
  • ftl/FTLLowerDFGToLLVM.cpp:

(JSC::FTL::LowerDFGToLLVM::doubleToInt32):
(JSC::FTL::LowerDFGToLLVM::doubleToUInt32):
(JSC::FTL::LowerDFGToLLVM::sensibleDoubleToInt32):

  • ftl/FTLOutput.h:

(JSC::FTL::Output::insertElement):
(JSC::FTL::Output::hasSensibleDoubleToInt):
(JSC::FTL::Output::sensibleDoubleToInt):

LayoutTests:

Reviewed by Michael Saboff.

  • js/regress/double-to-int32-typed-array-expected.txt: Added.
  • js/regress/double-to-int32-typed-array-no-inline-expected.txt: Added.
  • js/regress/double-to-int32-typed-array-no-inline.html: Added.
  • js/regress/double-to-int32-typed-array.html: Added.
  • js/regress/double-to-uint32-typed-array-expected.txt: Added.
  • js/regress/double-to-uint32-typed-array-no-inline-expected.txt: Added.
  • js/regress/double-to-uint32-typed-array-no-inline.html: Added.
  • js/regress/double-to-uint32-typed-array.html: Added.
  • js/regress/script-tests/double-to-int32-typed-array-no-inline.js: Added.

(foo):
(test):

  • js/regress/script-tests/double-to-int32-typed-array.js: Added.

(foo):
(test):

  • js/regress/script-tests/double-to-uint32-typed-array-no-inline.js: Added.

(foo):
(test):

  • js/regress/script-tests/double-to-uint32-typed-array.js: Added.

(foo):
(test):

Location:
trunk
Files:
12 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r160200 r160205  
     12013-12-04  Filip Pizlo  <fpizlo@apple.com>
     2
     3        FTL should use cvttsd2si directly for double-to-int32 conversions
     4        https://bugs.webkit.org/show_bug.cgi?id=125275
     5
     6        Reviewed by Michael Saboff.
     7
     8        * js/regress/double-to-int32-typed-array-expected.txt: Added.
     9        * js/regress/double-to-int32-typed-array-no-inline-expected.txt: Added.
     10        * js/regress/double-to-int32-typed-array-no-inline.html: Added.
     11        * js/regress/double-to-int32-typed-array.html: Added.
     12        * js/regress/double-to-uint32-typed-array-expected.txt: Added.
     13        * js/regress/double-to-uint32-typed-array-no-inline-expected.txt: Added.
     14        * js/regress/double-to-uint32-typed-array-no-inline.html: Added.
     15        * js/regress/double-to-uint32-typed-array.html: Added.
     16        * js/regress/script-tests/double-to-int32-typed-array-no-inline.js: Added.
     17        (foo):
     18        (test):
     19        * js/regress/script-tests/double-to-int32-typed-array.js: Added.
     20        (foo):
     21        (test):
     22        * js/regress/script-tests/double-to-uint32-typed-array-no-inline.js: Added.
     23        (foo):
     24        (test):
     25        * js/regress/script-tests/double-to-uint32-typed-array.js: Added.
     26        (foo):
     27        (test):
     28
    1292013-12-05  Bear Travis  <betravis@adobe.com>
    230

  • trunk/Source/JavaScriptCore/ChangeLog

    r160204 r160205  
     12013-12-04  Filip Pizlo  <fpizlo@apple.com>
     2
     3        FTL should use cvttsd2si directly for double-to-int32 conversions
     4        https://bugs.webkit.org/show_bug.cgi?id=125275
     5
     6        Reviewed by Michael Saboff.
     7       
     8        Wow. This was an ordeal. Using cvttsd2si was actually easy, but I learned, and
     9        sometimes even fixed, some interesting things:
     10       
     11        - The llvm.x86.sse2.cvttsd2si intrinsic can actually result in LLVM emitting a
     12          vcvttsd2si. I guess the intrinsic doesn't actually imply the instruction.
     13       
     14        - That whole thing about branchTruncateDoubleToUint32? Yeah we don't need that. It's
     15          better to use branchTruncateDoubleToInt32 instead. It has the right semantics for
     16          all of its callers (err, its one-and-only caller), and it's more likely to take
     17          fast path. This patch kills branchTruncateDoubleToUint32.
     18       
     19        - "a[i] = v; v = a[i]". Does this change v? OK, assume that 'a[i]' is a pure-ish
     20          operation - like an array access with 'i' being an integer index and we're not
     21          having a bad time. Now does this change v? CSE assumes that it doesn't. That's
     22          wrong. If 'a' is a typed array - the most sensible and pure kind of array - then
     23          this can be a truncating cast. For example 'v' could be a double and 'a' could be
     24          an integer array.
     25       
     26        - "v1 = a[i]; v2 = a[i]". Is v1 === v2 assuming that 'a[i]' is pure-ish? The answer
     27          is no. You could have a different arrayMode in each access. I know this sounds
     28          weird, but with concurrent JIT that might happen.
     29       
     30        This patch adds tests for all of this stuff, except for the first issue (it's weird
     31        but probably doesn't matter) and the last issue (it's too much of a freakshow).
     32
     33        * assembler/MacroAssemblerARM64.h:
     34        * assembler/MacroAssemblerARMv7.h:
     35        * assembler/MacroAssemblerX86Common.h:
     36        * dfg/DFGCSEPhase.cpp:
     37        (JSC::DFG::CSEPhase::getByValLoadElimination):
     38        (JSC::DFG::CSEPhase::performNodeCSE):
     39        * dfg/DFGSpeculativeJIT.cpp:
     40        (JSC::DFG::SpeculativeJIT::compilePutByValForIntTypedArray):
     41        * ftl/FTLAbbreviations.h:
     42        (JSC::FTL::vectorType):
     43        (JSC::FTL::getUndef):
     44        (JSC::FTL::buildInsertElement):
     45        * ftl/FTLIntrinsicRepository.h:
     46        * ftl/FTLLowerDFGToLLVM.cpp:
     47        (JSC::FTL::LowerDFGToLLVM::doubleToInt32):
     48        (JSC::FTL::LowerDFGToLLVM::doubleToUInt32):
     49        (JSC::FTL::LowerDFGToLLVM::sensibleDoubleToInt32):
     50        * ftl/FTLOutput.h:
     51        (JSC::FTL::Output::insertElement):
     52        (JSC::FTL::Output::hasSensibleDoubleToInt):
     53        (JSC::FTL::Output::sensibleDoubleToInt):
     54
    1552013-12-05  Commit Queue  <commit-queue@webkit.org>
    256

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

    r160056 r160205  
    11971197    }
    11981198
    1199     Jump branchTruncateDoubleToUint32(FPRegisterID src, RegisterID dest, BranchTruncateType branchType = BranchIfTruncateFailed)
    1200     {
    1201         // Truncate to a 64-bit integer in dataTempRegister, copy the low 32-bit to dest.
    1202         m_assembler.fcvtzs<64, 64>(dest, src);
    1203         // Check thlow 32-bits zero extend to be equal to the full value.
    1204         m_assembler.cmp<64>(dest, dest, ARM64Assembler::UXTW, 0);
    1205         return Jump(makeBranch(branchType == BranchIfTruncateSuccessful ? Equal : NotEqual));
    1206     }
    1207 
    12081199    void convertDoubleToFloat(FPRegisterID src, FPRegisterID dest)
    12091200    {

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

    r159564 r160205  
    10751075    }
    10761076
    1077     Jump branchTruncateDoubleToUint32(FPRegisterID src, RegisterID dest, BranchTruncateType branchType = BranchIfTruncateFailed)
    1078     {
    1079         m_assembler.vcvt_floatingPointToSigned(fpTempRegisterAsSingle(), src);
    1080         m_assembler.vmov(dest, fpTempRegisterAsSingle());
    1081        
    1082         Jump overflow = branch32(Equal, dest, TrustedImm32(0x7fffffff));
    1083         Jump success = branch32(GreaterThanOrEqual, dest, TrustedImm32(0));
    1084         overflow.link(this);
    1085 
    1086         if (branchType == BranchIfTruncateSuccessful)
    1087             return success;
    1088        
    1089         Jump failure = jump();
    1090         success.link(this);
    1091         return failure;
    1092     }
    1093 
    10941077    // Result is undefined if the value is outside of the integer range.
    10951078    void truncateDoubleToInt32(FPRegisterID src, RegisterID dest)

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h

    r159855 r160205  
    884884        m_assembler.cvttsd2si_rr(src, dest);
    885885        return branch32(branchType ? NotEqual : Equal, dest, TrustedImm32(0x80000000));
    886     }
    887 
    888     Jump branchTruncateDoubleToUint32(FPRegisterID src, RegisterID dest, BranchTruncateType branchType = BranchIfTruncateFailed)
    889     {
    890         ASSERT(isSSE2Present());
    891         m_assembler.cvttsd2si_rr(src, dest);
    892         return branch32(branchType ? GreaterThanOrEqual : LessThan, dest, TrustedImm32(0));
    893886    }
    894887

  • trunk/Source/JavaScriptCore/dfg/DFGCSEPhase.cpp

    r159886 r160205  
    332332    }
    333333   
    334     Node* getByValLoadElimination(Node* child1, Node* child2)
     334    Node* getByValLoadElimination(Node* child1, Node* child2, ArrayMode arrayMode)
    335335    {
    336336        for (unsigned i = m_indexInBlock; i--;) {
    … …  
    343343                if (!m_graph.byValIsPure(node))
    344344                    return 0;
    345                 if (node->child1() == child1 && node->child2() == child2)
     345                if (node->child1() == child1
     346                    && node->child2() == child2
     347                    && node->arrayMode().type() == arrayMode.type())
    346348                    return node;
    347349                break;
    … …  
    352354                if (!m_graph.byValIsPure(node))
    353355                    return 0;
    354                 if (m_graph.varArgChild(node, 0) == child1 && m_graph.varArgChild(node, 1) == child2)
     356                // Typed arrays
     357                if (arrayMode.typedArrayType() != NotTypedArray)
     358                    return 0;
     359                if (m_graph.varArgChild(node, 0) == child1
     360                    && m_graph.varArgChild(node, 1) == child2
     361                    && node->arrayMode().type() == arrayMode.type())
    355362                    return m_graph.varArgChild(node, 2).node();
    356363                // We must assume that the PutByVal will clobber the location we're getting from.
    … …  
    12171224                break;
    12181225            if (m_graph.byValIsPure(node))
    1219                 setReplacement(getByValLoadElimination(node->child1().node(), node->child2().node()));
     1226                setReplacement(getByValLoadElimination(node->child1().node(), node->child2().node(), node->arrayMode()));
    12201227            break;
    12211228               
    … …  
    12271234            Edge child2 = m_graph.varArgChild(node, 1);
    12281235            if (node->arrayMode().canCSEStorage()) {
    1229                 Node* replacement = getByValLoadElimination(child1.node(), child2.node());
     1236                Node* replacement = getByValLoadElimination(child1.node(), child2.node(), node->arrayMode());
    12301237                if (!replacement)
    12311238                    break;

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r160150 r160205  
    25072507                notNaN.link(&m_jit);
    25082508               
    2509                 MacroAssembler::Jump failed;
    2510                 if (isSigned(type))
    2511                     failed = m_jit.branchTruncateDoubleToInt32(fpr, gpr, MacroAssembler::BranchIfTruncateFailed);
    2512                 else
    2513                     failed = m_jit.branchTruncateDoubleToUint32(fpr, gpr, MacroAssembler::BranchIfTruncateFailed);
     2509                MacroAssembler::Jump failed = m_jit.branchTruncateDoubleToInt32(
     2510                    fpr, gpr, MacroAssembler::BranchIfTruncateFailed);
    25142511               
    25152512                addSlowPathGenerator(slowPathCall(failed, this, toInt32, gpr, fpr));

  • trunk/Source/JavaScriptCore/ftl/FTLAbbreviations.h

    r159545 r160205  
    5959
    6060static inline LType pointerType(LType type) { return llvm->PointerType(type, 0); }
     61static inline LType vectorType(LType type, unsigned count) { return llvm->VectorType(type, count); }
    6162
    6263enum PackingMode { NotPacked, Packed };
    … …  
    141142
    142143static inline LValue getParam(LValue function, unsigned index) { return llvm->GetParam(function, index); }
     144static inline LValue getUndef(LType type) { return llvm->GetUndef(type); }
    143145
    144146enum BitExtension { ZeroExtend, SignExtend };
    … …  
    218220static inline LValue buildICmp(LBuilder builder, LIntPredicate cond, LValue left, LValue right) { return llvm->BuildICmp(builder, cond, left, right, ""); }
    219221static inline LValue buildFCmp(LBuilder builder, LRealPredicate cond, LValue left, LValue right) { return llvm->BuildFCmp(builder, cond, left, right, ""); }
     222static inline LValue buildInsertElement(LBuilder builder, LValue vector, LValue element, LValue index) { return llvm->BuildInsertElement(builder, vector, element, index, ""); }
    220223
    221224enum SynchronizationScope { SingleThread, CrossThread };

  • trunk/Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h

    r159798 r160205  
    4343    macro(mulWithOverflow32, "llvm.smul.with.overflow.i32", functionType(structType(m_context, int32, boolean), int32, int32)) \
    4444    macro(mulWithOverflow64, "llvm.smul.with.overflow.i64", functionType(structType(m_context, int64, boolean), int64, int64)) \
    45     macro(subWithOverflow32, "llvm.ssub.with.overflow.i32", functionType(structType(m_context, int32, boolean), int32, int32)) \
    46     macro(subWithOverflow64, "llvm.ssub.with.overflow.i64", functionType(structType(m_context, int64, boolean), int64, int64)) \
    4745    macro(patchpointInt64, "llvm.experimental.patchpoint.i64", functionType(int64, int32, int32, ref8, int32, Variadic)) \
    4846    macro(patchpointVoid, "llvm.experimental.patchpoint.void", functionType(voidType, int32, int32, ref8, int32, Variadic)) \
    4947    macro(stackmap, "llvm.experimental.stackmap", functionType(voidType, int32, int32, Variadic)) \
    50     macro(trap, "llvm.trap", functionType(voidType))
     48    macro(subWithOverflow32, "llvm.ssub.with.overflow.i32", functionType(structType(m_context, int32, boolean), int32, int32)) \
     49    macro(subWithOverflow64, "llvm.ssub.with.overflow.i64", functionType(structType(m_context, int64, boolean), int64, int64)) \
     50    macro(trap, "llvm.trap", functionType(voidType)) \
     51    macro(x86SSE2CvtTSD2SI, "llvm.x86.sse2.cvttsd2si", functionType(int32, vectorType(doubleType, 2)))
    5152
    5253#define FOR_EACH_FUNCTION_TYPE(macro) \

  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp

    r160150 r160205  
    30123012    LValue doubleToInt32(LValue doubleValue)
    30133013    {
     3014        if (Output::hasSensibleDoubleToInt())
     3015            return sensibleDoubleToInt32(doubleValue);
     3016       
    30143017        double limit = pow(2, 31) - 1;
    30153018        return doubleToInt32(doubleValue, -limit, limit);
    … …  
    30183021    LValue doubleToUInt32(LValue doubleValue)
    30193022    {
     3023        if (Output::hasSensibleDoubleToInt())
     3024            return sensibleDoubleToInt32(doubleValue);
     3025       
    30203026        return doubleToInt32(doubleValue, 0, pow(2, 32) - 1, false);
     3027    }
     3028   
     3029    LValue sensibleDoubleToInt32(LValue doubleValue)
     3030    {
     3031        LBasicBlock slowPath = FTL_NEW_BLOCK(m_out, ("sensible doubleToInt32 slow path"));
     3032        LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("sensible doubleToInt32 continuation"));
     3033       
     3034        ValueFromBlock fastResult = m_out.anchor(
     3035            m_out.sensibleDoubleToInt(doubleValue));
     3036        m_out.branch(
     3037            m_out.equal(fastResult.value(), m_out.constInt32(0x80000000)),
     3038            slowPath, continuation);
     3039       
     3040        LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
     3041        ValueFromBlock slowResult = m_out.anchor(
     3042            m_out.call(m_out.operation(toInt32), doubleValue));
     3043        m_out.jump(continuation);
     3044       
     3045        m_out.appendTo(continuation, lastNext);
     3046        return m_out.phi(m_out.int32, fastResult, slowResult);
    30213047    }
    30223048   

  • trunk/Source/JavaScriptCore/ftl/FTLOutput.h

    r159545 r160205  
    155155    LValue bitNot(LValue value) { return buildNot(m_builder, value); }
    156156   
     157    LValue insertElement(LValue vector, LValue element, LValue index) { return buildInsertElement(m_builder, vector, element, index); }
     158   
    157159    LValue addWithOverflow32(LValue left, LValue right)
    158160    {
    … …  
    182184    {
    183185        return call(doubleAbsIntrinsic(), value);
     186    }
     187   
     188    static bool hasSensibleDoubleToInt() { return isX86(); }
     189    LValue sensibleDoubleToInt(LValue value)
     190    {
     191        RELEASE_ASSERT(isX86());
     192        return call(
     193            x86SSE2CvtTSD2SIIntrinsic(),
     194            insertElement(
     195                insertElement(getUndef(vectorType(doubleType, 2)), value, int32Zero),
     196                doubleZero, int32One));
    184197    }
    185198   
Note: See TracChangeset for help on using the changeset viewer.