Prechádzať zdrojové kódy

Address code review issues
Combined typedarray and array testing
Extended memset invariant test to check if calling the function with an invariant of a different type breaks the behavior.

Michael Ferris 10 rokov pred
rodič
commit
29b6bc76c8

+ 2 - 2
lib/Backend/GlobOpt.cpp

@@ -4117,7 +4117,7 @@ GlobOpt::IsAllowedForMemOpt(IR::Instr* instr, bool isMemset, IR::RegOpnd *baseOp
             !(
                 baseValueType.IsNativeIntArray() ||
                 // Memset allows native float and float32/float64 typed arrays as well. Todo:: investigate if memcopy can be done safely on float arrays
-                (isMemset ? (baseValueType.IsNativeFloatArray() || baseValueType.IsJITOptimizedTypedArray()) : baseValueType.IsTypedIntArray()) ||
+                (isMemset ? (baseValueType.IsNativeFloatArray() || baseValueType.IsTypedIntOrFloatArray()) : baseValueType.IsTypedIntArray()) ||
                 baseValueType.IsArray()
             )
         )
@@ -4252,7 +4252,7 @@ GlobOpt::CollectMemsetStElementI(IR::Instr *instr, Loop *loop)
     if (srcDef->IsRegOpnd())
     {
         IR::RegOpnd* opnd = srcDef->AsRegOpnd();
-        if (this->OptIsInvariant(opnd, this->currentBlock, loop, this->FindValue(opnd->m_sym), false, true))
+        if (this->OptIsInvariant(opnd, this->currentBlock, loop, this->FindValue(opnd->m_sym), true, true))
         {
             StackSym* sym = opnd->GetStackSym();
             if (sym->GetType() != TyVar)

+ 1 - 1
lib/Runtime/Language/ValueType.cpp

@@ -634,7 +634,7 @@ bool ValueType::IsLikelyTypedArray() const
     return IsLikelyObject() && GetObjectType() >= ObjectType::Int8Array && GetObjectType() <= ObjectType::CharArray;
 }
 
-bool ValueType::IsJITOptimizedTypedArray() const
+bool ValueType::IsTypedIntOrFloatArray() const
 {
     return IsObject() && ((GetObjectType() >= ObjectType::Int8Array  && GetObjectType() <= ObjectType::Float64Array));
 }

+ 1 - 1
lib/Runtime/Language/ValueType.h

@@ -211,7 +211,7 @@ public:
     bool IsTypedIntArray() const;
     bool IsLikelyTypedIntArray() const;
 
-    bool IsJITOptimizedTypedArray() const;
+    bool IsTypedIntOrFloatArray() const;
 
     bool IsOptimizedTypedArray() const;
     bool IsLikelyOptimizedTypedArray() const;

+ 73 - 28
test/Array/memset_invariant.js

@@ -3,50 +3,95 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 
-function makeTest(name, v1, v2) {
+function* makeValueGen(a, b) {
+  // return a for profiling
+  yield a;
+  // return b to bailout
+  yield b;
+  // return b again to compare with non jit function
+  yield b;
+}
+
+function makeTest(name, config) {
   const f1 = `function ${name}(arr) {
-    for(var i = 0; i < 50; ++i) {arr[i] = ${v1};}
+    for(var i = -5; i < 15; ++i) {arr[i] = ${config.v1};}
+    return arr;
+  }`;
+  const f2 = customName => `function ${customName}P(arr, v) {
+    for(var i = 1; i < 8; ++i) {arr[i] = v;}
     return arr;
   }`;
-  const f2 = `function ${name}V(v, arr) {
-    for(var i = 0; i < 10; ++i) {arr[i] = v;}
+  const f3 = `function ${name}V(arr) {
+    const v = ${config.v1};
+    for(var i = -2; i < 17; ++i) {arr[i] = v;}
     return arr;
-  }.bind(null, ${v2 !== undefined ? v2 : v1});`;
+  }`;
+
+  const extraTests = (config.wrongTypes || []).map((wrongType, i) => {
+    const difValue = {f: f2(`${name}W${i}`), compare: f2(`${name}WC${i}`)};
+    const genValue = makeValueGen(config.v1, wrongType);
+    Reflect.defineProperty(difValue, "v", {
+      get: () => genValue.next().value
+    });
+    return difValue;
+  });
+
+  const tests = [
+    {f: f1},
+    {f: f2(name), v: config.v2 !== undefined ? config.v2 : config.v1},
+    {f: f3},
+  ].concat(extraTests);
 
-  return [f1, f2].map(fnText => {
+  const convertTest = function(fnText) {
     var fn;
     eval(`fn = ${fnText}`);
     return fn;
-  });
+  };
+  for(const t of tests) {
+    t.f = convertTest(t.f);
+    t.compare = t.compare && convertTest(t.compare);
+  }
+  return tests;
 }
 
+const allTypes = [0, 1.5, undefined, null, 9223372036854775807, "string", {a: null, b: "b"}];
 const tests = [
-  {name: "memsetUndefined", v1: undefined},
-  {name: "memsetNull", v1: null},
-  {name: "memsetFloat", v1: 3.14, v2: -87.684},
-  {name: "memsetNumber", v1: 9223372036854775807, v2: -987654987654987},
-  {name: "memsetBoolean", v1: true, v2: false},
-  {name: "memsetString", v1: "\"thatString\"", v2: "`A template string`"},
-  {name: "memset8bit", v1: "16&255", v2: "16&255"},
+  {name: "memsetUndefined", v1: undefined, wrongTypes: allTypes},
+  {name: "memsetNull", v1: null, wrongTypes: allTypes},
+  {name: "memsetFloat", v1: 3.14, v2: -87.684, wrongTypes: allTypes},
+  {name: "memsetNumber", v1: 9223372036854775807, v2: -987654987654987, wrongTypes: allTypes},
+  {name: "memsetBoolean", v1: true, v2: false, wrongTypes: allTypes},
+  {name: "memsetString", v1: "\"thatString\"", v2: "`A template string`", wrongTypes: allTypes},
+  {name: "memsetObject", v1: "{test: 1}", v2: [1, 2, 3], wrongTypes: allTypes},
 ];
 
+const types = "Int8Array Uint8Array Int16Array Uint16Array Int32Array Uint32Array Float32Array Float64Array Array".split(" ");
+const global = this;
+
 let passed = true;
 for(const test of tests) {
-  const testName = test.name;
-  const fns = makeTest(testName, test.v1, test.v2);
-  for(const fn of fns) {
-    const a1 = fn(new Array(10));
-    const a2 = fn(new Array(10));
-    if(a1.length !== a2.length) {
-      passed = false;
-      print(`${fn.name} (${t}) didn't return arrays with same length`);
-      continue;
-    }
-    for(let i = 0; i < a1.length; ++i) {
-      if(a1[i] !== a2[i] && !(isNaN(a1[i]) && isNaN(a2[i]))) {
+  for(const t of types) {
+    const fns = makeTest(`${test.name}${t}`, test);
+    for(const detail of fns) {
+      const fn = detail.f;
+      let a1 = fn(new global[t](10), detail.v);
+      const a2 = fn(new global[t](10), detail.v);
+      if(detail.compare) {
+        // the optimized version ran with a different value. Run again with a clean function to compare
+        // reuse a1 to test if we correctly overwrite the values
+        a1 = detail.compare(a1, detail.v);
+      }
+      if(a1.length !== a2.length) {
         passed = false;
-        print(`${fn.name} (${t}): a1[${i}](${a1[i]}) != a2[${i}](${a2[i]})`);
-        break;
+        print(`${fn.name} (${t}) didn't return arrays with same length`);
+        continue;
+      }
+      for(let i = 0; i < a1.length; ++i) {
+        if(a1[i] !== a2[i] && !(isNaN(a1[i]) && isNaN(a2[i]))) {
+          passed = false;
+          print(`${fn.name} (${t}): a1[${i}](${a1[i]}) != a2[${i}](${a2[i]})`);
+          break;
+        }
       }
     }
   }

+ 1 - 1
test/Array/rlexe.xml

@@ -628,7 +628,7 @@
   <test>
      <default>
         <files>memset_invariant.js</files>
-        <compile-flags>-mic:1 -off:simplejit -mmoc:0</compile-flags>
+        <compile-flags>-mic:1 -off:simplejit -mmoc:0 -off:JITLoopBody</compile-flags>
      </default>
   </test>
   <test>

+ 0 - 58
test/typedarray/memset_invariant.js

@@ -1,58 +0,0 @@
-//-------------------------------------------------------------------------------------------------------
-// Copyright (C) Microsoft. All rights reserved.
-// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
-//-------------------------------------------------------------------------------------------------------
-
-function makeTest(name, v1, v2) {
-  const f1 = `function ${name}(arr) {
-    for(var i = -5; i < 15; ++i) {arr[i] = ${v1};}
-    return arr;
-  }`;
-  const f2 = `function ${name}V(v, arr) {
-    for(var i = -875; i < 485; ++i) {arr[i] = v;}
-    return arr;
-  }.bind(null, ${v2 !== undefined ? v2 : v1});`;
-
-  return [f1, f2].map(fnText => {
-    var fn;
-    eval(`fn = ${fnText}`);
-    return fn;
-  });
-}
-
-const tests = [
-  {name: "memsetUndefined", v1: undefined},
-  {name: "memsetNull", v1: null},
-  {name: "memsetFloat", v1: 3.14, v2: -87.684},
-  {name: "memsetNumber", v1: 9223372036854775807, v2: -987654987654987},
-  {name: "memsetBoolean", v1: true, v2: false},
-  {name: "memsetString", v1: "\"thatString\"", v2: "`A template string`"},
-];
-
-const types = "Int8Array Uint8Array Int16Array Uint16Array Int32Array Uint32Array Float32Array Float64Array".split(" ");
-const global = this;
-
-let passed = true;
-for(const test of tests) {
-  for(const t of types) {
-    const testName = test.name + t;
-    const fns = makeTest(testName, test.v1, test.v2);
-    for(const fn of fns) {
-      const a1 = fn(new global[t](10));
-      const a2 = fn(new global[t](10));
-      if(a1.length !== a2.length) {
-        passed = false;
-        print(`${fn.name} (${t}) didn't return arrays with same length`);
-        continue;
-      }
-      for(let i = 0; i < a1.length; ++i) {
-        if(a1[i] !== a2[i] && !(isNaN(a1[i]) && isNaN(a2[i]))) {
-          passed = false;
-          print(`${fn.name} (${t}): a1[${i}](${a1[i]}) != a2[${i}](${a2[i]})`);
-          break;
-        }
-      }
-    }
-  }
-}
-print(passed ? "PASSED" : "FAILED");

+ 0 - 6
test/typedarray/rlexe.xml

@@ -304,12 +304,6 @@ Below test fails with difference in space. Investigate the cause and re-enable t
       <compile-flags>-mic:1 -off:simplejit -off:JITLoopBody -mmoc:0</compile-flags>
     </default>
   </test>
-  <test>
-    <default>
-      <files>memset_invariant.js</files>
-      <compile-flags>-mic:1 -off:simplejit -off:JITLoopBody -mmoc:0</compile-flags>
-    </default>
-  </test>
   <test>
     <default>
       <files>memcopy.js</files>