Changeset 103637 in webkit


Ignore:
Timestamp:
Dec 23, 2011 1:08:12 PM (12 years ago)
Author:
fpizlo@apple.com
Message:

DFG loads from signed 8-bit and 16-bit typed arrays are broken
​https://bugs.webkit.org/show_bug.cgi?id=75163

Source/JavaScriptCore:

Reviewed by Geoffrey Garen.

Added 8-bit and 16-bit signed loads. Because doing so on ARM is less trivial, I'm
currently disabling Int8Array and Int16Array optimizations on ARM.

  • assembler/MacroAssemblerX86Common.h:

(JSC::MacroAssemblerX86Common::load8Signed):
(JSC::MacroAssemblerX86Common::load16Signed):

  • assembler/X86Assembler.h:

(JSC::X86Assembler::movswl_mr):
(JSC::X86Assembler::movsbl_mr):

  • bytecode/PredictedType.h:

(JSC::isActionableMutableArrayPrediction):

  • dfg/DFGNode.h:

(JSC::DFG::Node::shouldSpeculateInt8Array):
(JSC::DFG::Node::shouldSpeculateInt16Array):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):

LayoutTests:

Reviewed by Geoffrey Garen.

Fixed some minor goofs in the previously comitted typed array tests, and added
new ones to cover this bug.

  • fast/js/dfg-int16array-expected.txt: Added.
  • fast/js/dfg-int16array.html: Added.
  • fast/js/dfg-int8array-expected.txt: Added.
  • fast/js/dfg-int8array.html: Added.
  • fast/js/script-tests/dfg-float32array.js:

(getters.getter1.a):
(.a):
(setters.setter1.a):
(safeGetter):

  • fast/js/script-tests/dfg-int16array.js: Added.

(getter1):
(setter1):
(getter2):
(setter2):
(getter3):
(setter3):
(getter4):
(setter4):
(getters.getter1.a):
(.a):
(setters.setter1.a):
(safeGetter):
(safeSetter):

  • fast/js/script-tests/dfg-int32array.js:

(getters.getter1.a):
(.a):
(setters.setter1.a):
(safeGetter):

  • fast/js/script-tests/dfg-int8array.js: Added.

(getter1):
(setter1):
(getter2):
(setter2):
(getter3):
(setter3):
(getter4):
(setter4):
(getters.getter1.a):
(.a):
(setters.setter1.a):
(safeGetter):
(safeSetter):

Location:
trunk
Files:
4 added
9 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r103636 r103637  
     12011-12-23  Filip Pizlo  <fpizlo@apple.com>
     2
     3        DFG loads from signed 8-bit and 16-bit typed arrays are broken
     4        https://bugs.webkit.org/show_bug.cgi?id=75163
     5
     6        Reviewed by Geoffrey Garen.
     7       
     8        Fixed some minor goofs in the previously comitted typed array tests, and added
     9        new ones to cover this bug.
     10
     11        * fast/js/dfg-int16array-expected.txt: Added.
     12        * fast/js/dfg-int16array.html: Added.
     13        * fast/js/dfg-int8array-expected.txt: Added.
     14        * fast/js/dfg-int8array.html: Added.
     15        * fast/js/script-tests/dfg-float32array.js:
     16        (getters.getter1.a):
     17        (.a):
     18        (setters.setter1.a):
     19        (safeGetter):
     20        * fast/js/script-tests/dfg-int16array.js: Added.
     21        (getter1):
     22        (setter1):
     23        (getter2):
     24        (setter2):
     25        (getter3):
     26        (setter3):
     27        (getter4):
     28        (setter4):
     29        (getters.getter1.a):
     30        (.a):
     31        (setters.setter1.a):
     32        (safeGetter):
     33        (safeSetter):
     34        * fast/js/script-tests/dfg-int32array.js:
     35        (getters.getter1.a):
     36        (.a):
     37        (setters.setter1.a):
     38        (safeGetter):
     39        * fast/js/script-tests/dfg-int8array.js: Added.
     40        (getter1):
     41        (setter1):
     42        (getter2):
     43        (setter2):
     44        (getter3):
     45        (setter3):
     46        (getter4):
     47        (setter4):
     48        (getters.getter1.a):
     49        (.a):
     50        (setters.setter1.a):
     51        (safeGetter):
     52        (safeSetter):
     53
    1542011-12-23  Filip Pizlo  <fpizlo@apple.com>
    255

  • trunk/LayoutTests/fast/js/script-tests/dfg-float32array.js

    r103594 r103637  
    5656
    5757var True = true;
     58var Empty = "";
    5859
    5960var getters = [
    6061    getter1,
    61     function(a, b) { a = {f:a}; return eval("getter2(a, b)"); },
    62     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter3(a, b)"); },
    63     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter4(True, a, b)"); }
     62    function(a, b) { a = {f:a}; return eval(Empty + "getter2(a, b)"); },
     63    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter3(a, b)"); },
     64    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter4(True, a, b)"); }
    6465];
    6566var setters = [
    6667    setter1,
    67     function(a, b, c) { a = {f:a}; return eval("setter2(a, b, c)"); },
    68     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter3(a, b, c)"); },
    69     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter4(True, a, b, c)"); }
     68    function(a, b, c) { a = {f:a}; return eval(Empty + "setter2(a, b, c)"); },
     69    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter3(a, b, c)"); },
     70    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter4(True, a, b, c)"); }
    7071];
    7172
    7273function safeGetter(a, b) {
    73     return eval("a[\"\" + " + b + "] = c");
     74    return eval("a[\"\" + " + b + "]");
    7475}
    7576

  • trunk/LayoutTests/fast/js/script-tests/dfg-int16array.js

    r103636 r103637  
    5656
    5757var True = true;
     58var Empty = "";
    5859
    5960var getters = [
    6061    getter1,
    61     function(a, b) { a = {f:a}; return eval("getter2(a, b)"); },
    62     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter3(a, b)"); },
    63     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter4(True, a, b)"); }
     62    function(a, b) { a = {f:a}; return eval(Empty + "getter2(a, b)"); },
     63    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter3(a, b)"); },
     64    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter4(True, a, b)"); }
    6465];
    6566var setters = [
    6667    setter1,
    67     function(a, b, c) { a = {f:a}; return eval("setter2(a, b, c)"); },
    68     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter3(a, b, c)"); },
    69     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter4(True, a, b, c)"); }
     68    function(a, b, c) { a = {f:a}; return eval(Empty + "setter2(a, b, c)"); },
     69    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter3(a, b, c)"); },
     70    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter4(True, a, b, c)"); }
    7071];
    7172
    7273function safeGetter(a, b) {
    73     return eval("a[\"\" + " + b + "] = c");
     74    return eval("a[\"\" + " + b + "]");
    7475}
    7576
    … …  
    7980
    8081for (var si = 0; si < setters.length; ++si) {
    81     var array = new Int32Array(101);
    82     var checkArray = new Int32Array(101);
     82    var array = new Int16Array(101);
     83    var checkArray = new Int16Array(101);
    8384    var indexOffset = 0;
    8485    var valueOffset = 0;
    … …  
    100101        var checkA = checkArray;
    101102        var b = (i % 100) + indexOffset;
    102         var c = i + valueOffset;
     103        var c = (i << 8) + i + valueOffset;
     104        if (i % 2)
     105            c = -c;
    103106       
    104107        setter(a, b, c);
    … …  
    109112
    110113for (var gi = 0; gi < getters.length; ++gi) {
    111     var array = new Int32Array(101);
     114    var array = new Int16Array(101);
    112115    var indexOffset = 0;
    113116    var valueOffset = 0;
    … …  
    126129        var a = array;
    127130        var b = (i % 100) + indexOffset;
    128         var c = i + valueOffset;
     131        var c = (i << 8) + i + valueOffset;
     132        if (i % 2)
     133            c = -c;
    129134       
    130135        safeSetter(a, b, c);

  • trunk/LayoutTests/fast/js/script-tests/dfg-int32array.js

    r103594 r103637  
    5656
    5757var True = true;
     58var Empty = "";
    5859
    5960var getters = [
    6061    getter1,
    61     function(a, b) { a = {f:a}; return eval("getter2(a, b)"); },
    62     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter3(a, b)"); },
    63     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter4(True, a, b)"); }
     62    function(a, b) { a = {f:a}; return eval(Empty + "getter2(a, b)"); },
     63    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter3(a, b)"); },
     64    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter4(True, a, b)"); }
    6465];
    6566var setters = [
    6667    setter1,
    67     function(a, b, c) { a = {f:a}; return eval("setter2(a, b, c)"); },
    68     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter3(a, b, c)"); },
    69     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter4(True, a, b, c)"); }
     68    function(a, b, c) { a = {f:a}; return eval(Empty + "setter2(a, b, c)"); },
     69    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter3(a, b, c)"); },
     70    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter4(True, a, b, c)"); }
    7071];
    7172
    7273function safeGetter(a, b) {
    73     return eval("a[\"\" + " + b + "] = c");
     74    return eval("a[\"\" + " + b + "]");
    7475}
    7576

  • trunk/LayoutTests/fast/js/script-tests/dfg-int8array.js

    r103636 r103637  
    5656
    5757var True = true;
     58var Empty = "";
    5859
    5960var getters = [
    6061    getter1,
    61     function(a, b) { a = {f:a}; return eval("getter2(a, b)"); },
    62     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter3(a, b)"); },
    63     function(a, b) { a = {f:a}; b = {f:b}; return eval("getter4(True, a, b)"); }
     62    function(a, b) { a = {f:a}; return eval(Empty + "getter2(a, b)"); },
     63    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter3(a, b)"); },
     64    function(a, b) { a = {f:a}; b = {f:b}; return eval(Empty + "getter4(True, a, b)"); }
    6465];
    6566var setters = [
    6667    setter1,
    67     function(a, b, c) { a = {f:a}; return eval("setter2(a, b, c)"); },
    68     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter3(a, b, c)"); },
    69     function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval("setter4(True, a, b, c)"); }
     68    function(a, b, c) { a = {f:a}; return eval(Empty + "setter2(a, b, c)"); },
     69    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter3(a, b, c)"); },
     70    function(a, b, c) { a = {f:a}; b = {f:b}; c = {f:c}; return eval(Empty + "setter4(True, a, b, c)"); }
    7071];
    7172
    7273function safeGetter(a, b) {
    73     return eval("a[\"\" + " + b + "] = c");
     74    return eval("a[\"\" + " + b + "]");
    7475}
    7576
    … …  
    7980
    8081for (var si = 0; si < setters.length; ++si) {
    81     var array = new Int32Array(101);
    82     var checkArray = new Int32Array(101);
     82    var array = new Int8Array(101);
     83    var checkArray = new Int8Array(101);
    8384    var indexOffset = 0;
    8485    var valueOffset = 0;
    … …  
    101102        var b = (i % 100) + indexOffset;
    102103        var c = i + valueOffset;
     104        if (i % 2)
     105            c = -c;
    103106       
    104107        setter(a, b, c);
    … …  
    109112
    110113for (var gi = 0; gi < getters.length; ++gi) {
    111     var array = new Int32Array(101);
     114    var array = new Int8Array(101);
    112115    var indexOffset = 0;
    113116    var valueOffset = 0;
    … …  
    127130        var b = (i % 100) + indexOffset;
    128131        var c = i + valueOffset;
     132        if (i % 2)
     133            c = -c;
    129134       
    130135        safeSetter(a, b, c);
    131         shouldBe("getter(a, b, c)", "" + safeGetter(a, b, c));
     136        shouldBe("getter(a, b, c)", "" + safeGetter(a, b));
    132137    }
    133138}

  • trunk/Source/JavaScriptCore/ChangeLog

    r103636 r103637  
     12011-12-23  Filip Pizlo  <fpizlo@apple.com>
     2
     3        DFG loads from signed 8-bit and 16-bit typed arrays are broken
     4        https://bugs.webkit.org/show_bug.cgi?id=75163
     5
     6        Reviewed by Geoffrey Garen.
     7       
     8        Added 8-bit and 16-bit signed loads. Because doing so on ARM is less trivial, I'm
     9        currently disabling Int8Array and Int16Array optimizations on ARM.
     10
     11        * assembler/MacroAssemblerX86Common.h:
     12        (JSC::MacroAssemblerX86Common::load8Signed):
     13        (JSC::MacroAssemblerX86Common::load16Signed):
     14        * assembler/X86Assembler.h:
     15        (JSC::X86Assembler::movswl_mr):
     16        (JSC::X86Assembler::movsbl_mr):
     17        * bytecode/PredictedType.h:
     18        (JSC::isActionableMutableArrayPrediction):
     19        * dfg/DFGNode.h:
     20        (JSC::DFG::Node::shouldSpeculateInt8Array):
     21        (JSC::DFG::Node::shouldSpeculateInt16Array):
     22        * dfg/DFGSpeculativeJIT.cpp:
     23        (JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
     24
    1252011-12-23  Filip Pizlo  <fpizlo@apple.com>
    226

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

    r103636 r103637  
    487487    }
    488488   
     489    void load8Signed(BaseIndex address, RegisterID dest)
     490    {
     491        m_assembler.movsbl_mr(address.offset, address.base, address.index, address.scale, dest);
     492    }
     493
     494    void load8Signed(ImplicitAddress address, RegisterID dest)
     495    {
     496        m_assembler.movsbl_mr(address.offset, address.base, dest);
     497    }
     498   
    489499    void load16(BaseIndex address, RegisterID dest)
    490500    {
    … …  
    495505    {
    496506        m_assembler.movzwl_mr(address.offset, address.base, dest);
     507    }
     508
     509    void load16Signed(BaseIndex address, RegisterID dest)
     510    {
     511        m_assembler.movswl_mr(address.offset, address.base, address.index, address.scale, dest);
     512    }
     513   
     514    void load16Signed(Address address, RegisterID dest)
     515    {
     516        m_assembler.movswl_mr(address.offset, address.base, dest);
    497517    }
    498518

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

    r101886 r103637  
    187187        OP2_IMUL_GvEv       = 0xAF,
    188188        OP2_MOVZX_GvEb      = 0xB6,
     189        OP2_MOVSX_GvEb      = 0xBE,
    189190        OP2_MOVZX_GvEw      = 0xB7,
     191        OP2_MOVSX_GvEw      = 0xBF,
    190192        OP2_PEXTRW_GdUdIb   = 0xC5,
    191193        OP2_PSLLQ_UdqIb     = 0x73,
    … …  
    12251227    }
    12261228
     1229    void movswl_mr(int offset, RegisterID base, RegisterID dst)
     1230    {
     1231        m_formatter.twoByteOp(OP2_MOVSX_GvEw, dst, base, offset);
     1232    }
     1233
     1234    void movswl_mr(int offset, RegisterID base, RegisterID index, int scale, RegisterID dst)
     1235    {
     1236        m_formatter.twoByteOp(OP2_MOVSX_GvEw, dst, base, index, scale, offset);
     1237    }
     1238
    12271239    void movzbl_mr(int offset, RegisterID base, RegisterID dst)
    12281240    {
    … …  
    12331245    {
    12341246        m_formatter.twoByteOp(OP2_MOVZX_GvEb, dst, base, index, scale, offset);
     1247    }
     1248
     1249    void movsbl_mr(int offset, RegisterID base, RegisterID dst)
     1250    {
     1251        m_formatter.twoByteOp(OP2_MOVSX_GvEb, dst, base, offset);
     1252    }
     1253   
     1254    void movsbl_mr(int offset, RegisterID base, RegisterID index, int scale, RegisterID dst)
     1255    {
     1256        m_formatter.twoByteOp(OP2_MOVSX_GvEb, dst, base, index, scale, offset);
    12351257    }
    12361258

  • trunk/Source/JavaScriptCore/bytecode/PredictedType.h

    r103604 r103637  
    156156    return isArrayPrediction(value)
    157157        || isByteArrayPrediction(value)
     158#if CPU(X86) || CPU(X86_64)
    158159        || isInt8ArrayPrediction(value)
    159160        || isInt16ArrayPrediction(value)
     161#endif
    160162        || isInt32ArrayPrediction(value)
    161163        || isUint8ArrayPrediction(value)

  • trunk/Source/JavaScriptCore/dfg/DFGNode.h

    r103604 r103637  
    932932    bool shouldSpeculateInt8Array()
    933933    {
     934#if CPU(X86) || CPU(X86_64)
    934935        return isInt8ArrayPrediction(prediction());
     936#else
     937        return false;
     938#endif
    935939    }
    936940   
    937941    bool shouldSpeculateInt16Array()
    938942    {
     943#if CPU(X86) || CPU(X86_64)
    939944        return isInt16ArrayPrediction(prediction());
     945#else
     946        return false;
     947#endif
    940948    }
    941949   

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

    r103636 r103637  
    16831683    switch (elementSize) {
    16841684    case 1:
    1685         m_jit.load8(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesOne), resultReg);
     1685        if (signedness == SignedTypedArray)
     1686            m_jit.load8Signed(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesOne), resultReg);
     1687        else
     1688            m_jit.load8(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesOne), resultReg);
    16861689        break;
    16871690    case 2:
    1688         m_jit.load16(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesTwo), resultReg);
     1691        if (signedness == SignedTypedArray)
     1692            m_jit.load16Signed(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesTwo), resultReg);
     1693        else
     1694            m_jit.load16(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesTwo), resultReg);
    16891695        break;
    16901696    case 4:
Note: See TracChangeset for help on using the changeset viewer.