Fix a register assignment bug in typed array stores without SSE3 available.

The old code used a separate HToInt32 instruction which had a wrong register
constraint for the input register which caused wrong result when the stored value
is used after a typed array store. (UseRegister instead of UseTempRegister) when no
SSE3 is available.

This change fixes it by replacing HToInt32 with the corresponding HChange
instruction which has correct register contraints.

TEST=mjsunit/compiler/regress-toint32.js
Review URL: https://chromiumcodereview.appspot.com/9565007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10891 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index fad8f4b..8654a3b 100644 (file)
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -1763,31 +1763,6 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) {
 }
 
 
-LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) {
-  HValue* value = instr->value();
-  Representation input_rep = value->representation();
-  LOperand* reg = UseRegister(value);
-  if (input_rep.IsDouble()) {
-    LOperand* temp1 = TempRegister();
-    LOperand* temp2 = TempRegister();
-    LDoubleToI* res = new(zone()) LDoubleToI(reg, temp1, temp2);
-    return AssignEnvironment(DefineAsRegister(res));
-  } else if (input_rep.IsInteger32()) {
-    // Canonicalization should already have removed the hydrogen instruction in
-    // this case, since it is a noop.
-    UNREACHABLE();
-    return NULL;
-  } else {
-    ASSERT(input_rep.IsTagged());
-    LOperand* temp1 = TempRegister();
-    LOperand* temp2 = TempRegister();
-    LOperand* temp3 = FixedTemp(d11);
-    LTaggedToI* res = new(zone()) LTaggedToI(reg, temp1, temp2, temp3);
-    return AssignEnvironment(DefineSameAsFirst(res));
-  }
-}
-
-
 LInstruction* LChunkBuilder::DoReturn(HReturn* instr) {
   return new(zone()) LReturn(UseFixed(instr->value(), r0));
 }

diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index cee90a9..eb0e0fd 100644 (file)
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -174,7 +174,6 @@ class LChunkBuilder;
   V(ThisFunction)                              \
   V(Throw)                                     \
   V(ToFastProperties)                          \
-  V(ToInt32)                                   \
   V(TransitionElementsKind)                    \
   V(Typeof)                                    \
   V(TypeofIsAndBranch)                         \
@@ -1243,34 +1242,6 @@ class HClampToUint8: public HUnaryOperation {
 };
 
 
-class HToInt32: public HUnaryOperation {
- public:
-  explicit HToInt32(HValue* value)
-      : HUnaryOperation(value) {
-    set_representation(Representation::Integer32());
-    SetFlag(kUseGVN);
-    SetFlag(kTruncatingToInt32);
-  }
-
-  virtual Representation RequiredInputRepresentation(int index) {
-    return Representation::None();
-  }
-
-  virtual HValue* Canonicalize() {
-    if (value()->representation().IsInteger32()) {
-      return value();
-    } else {
-      return this;
-    }
-  }
-
-  DECLARE_CONCRETE_INSTRUCTION(ToInt32)
-
- protected:
-  virtual bool DataEquals(HValue* other) { return true; }
-};
-
-
 class HSimulate: public HInstruction {
  public:
   HSimulate(int ast_id, int pop_count)

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 33fa0f4..cbcf32f 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4514,9 +4514,7 @@ HInstruction* HGraphBuilder::BuildExternalArrayElementAccess(
     ASSERT(val != NULL);
     switch (elements_kind) {
       case EXTERNAL_PIXEL_ELEMENTS: {
-        HClampToUint8* clamp = new(zone()) HClampToUint8(val);
-        AddInstruction(clamp);
-        val = clamp;
+        val = AddInstruction(new(zone()) HClampToUint8(val));
         break;
       }
       case EXTERNAL_BYTE_ELEMENTS:
@@ -4525,9 +4523,13 @@ HInstruction* HGraphBuilder::BuildExternalArrayElementAccess(
       case EXTERNAL_UNSIGNED_SHORT_ELEMENTS:
       case EXTERNAL_INT_ELEMENTS:
       case EXTERNAL_UNSIGNED_INT_ELEMENTS: {
-        HToInt32* floor_val = new(zone()) HToInt32(val);
-        AddInstruction(floor_val);
-        val = floor_val;
+        if (!val->representation().IsInteger32()) {
+          val = AddInstruction(new(zone()) HChange(
+              val,
+              Representation::Integer32(),
+              true,  // Truncate to int32.
+              false));  // Don't deoptimize undefined (irrelevant here).
+        }
         break;
       }
       case EXTERNAL_FLOAT_ELEMENTS:
@@ -4544,6 +4546,7 @@ HInstruction* HGraphBuilder::BuildExternalArrayElementAccess(
     return new(zone()) HStoreKeyedSpecializedArrayElement(
         external_elements, checked_key, val, elements_kind);
   } else {
+    ASSERT(val == NULL);
     return new(zone()) HLoadKeyedSpecializedArrayElement(
         external_elements, checked_key, elements_kind);
   }

diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc
index ef71836..94e992f 100644 (file)
--- a/src/ia32/lithium-ia32.cc
+++ b/src/ia32/lithium-ia32.cc
@@ -1816,34 +1816,6 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) {
 }
 
 
-LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) {
-  HValue* value = instr->value();
-  Representation input_rep = value->representation();
-
-  LInstruction* result;
-  if (input_rep.IsDouble()) {
-    LOperand* reg = UseRegister(value);
-    LOperand* temp_reg =
-        CpuFeatures::IsSupported(SSE3) ? NULL : TempRegister();
-    result = DefineAsRegister(new(zone()) LDoubleToI(reg, temp_reg));
-  } else if (input_rep.IsInteger32()) {
-    // Canonicalization should already have removed the hydrogen instruction in
-    // this case, since it is a noop.
-    UNREACHABLE();
-    return NULL;
-  } else {
-    ASSERT(input_rep.IsTagged());
-    LOperand* reg = UseRegister(value);
-    // Register allocator doesn't (yet) support allocation of double
-    // temps. Reserve xmm1 explicitly.
-    LOperand* xmm_temp =
-        CpuFeatures::IsSupported(SSE3) ? NULL : FixedTemp(xmm1);
-    result = DefineSameAsFirst(new(zone()) LTaggedToI(reg, xmm_temp));
-  }
-  return AssignEnvironment(result);
-}
-
-
 LInstruction* LChunkBuilder::DoReturn(HReturn* instr) {
   return new(zone()) LReturn(UseFixed(instr->value(), eax));
 }

diff --git a/src/mips/lithium-mips.cc b/src/mips/lithium-mips.cc
index 9ec1c1b..bd1014f 100644 (file)
--- a/src/mips/lithium-mips.cc
+++ b/src/mips/lithium-mips.cc
@@ -1766,31 +1766,6 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) {
 }
 
 
-LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) {
-  HValue* value = instr->value();
-  Representation input_rep = value->representation();
-  LOperand* reg = UseRegister(value);
-  if (input_rep.IsDouble()) {
-    LOperand* temp1 = TempRegister();
-    LOperand* temp2 = TempRegister();
-    LDoubleToI* res = new(zone()) LDoubleToI(reg, temp1, temp2);
-    return AssignEnvironment(DefineAsRegister(res));
-  } else if (input_rep.IsInteger32()) {
-    // Canonicalization should already have removed the hydrogen instruction in
-    // this case, since it is a noop.
-    UNREACHABLE();
-    return NULL;
-  } else {
-    ASSERT(input_rep.IsTagged());
-    LOperand* temp1 = TempRegister();
-    LOperand* temp2 = TempRegister();
-    LOperand* temp3 = FixedTemp(f22);
-    LTaggedToI* res = new(zone()) LTaggedToI(reg, temp1, temp2, temp3);
-    return AssignEnvironment(DefineSameAsFirst(res));
-  }
-}
-
-
 LInstruction* LChunkBuilder::DoReturn(HReturn* instr) {
   return new(zone()) LReturn(UseFixed(instr->value(), v0));
 }

diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index 45fda1a..8cddb36 100644 (file)
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -1750,32 +1750,6 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) {
 }
 
 
-LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) {
-  HValue* value = instr->value();
-  Representation input_rep = value->representation();
-  LOperand* reg = UseRegister(value);
-  if (input_rep.IsDouble()) {
-    return AssignEnvironment(DefineAsRegister(new(zone()) LDoubleToI(reg)));
-  } else if (input_rep.IsInteger32()) {
-    // Canonicalization should already have removed the hydrogen instruction in
-    // this case, since it is a noop.
-    UNREACHABLE();
-    return NULL;
-  } else {
-    ASSERT(input_rep.IsTagged());
-    LOperand* reg = UseRegister(value);
-    // Register allocator doesn't (yet) support allocation of double
-    // temps. Reserve xmm1 explicitly.
-    LOperand* xmm_temp =
-        CpuFeatures::IsSupported(SSE3)
-        ? NULL
-        : FixedTemp(xmm1);
-    return AssignEnvironment(
-        DefineSameAsFirst(new(zone()) LTaggedToI(reg, xmm_temp)));
-  }
-}
-
-
 LInstruction* LChunkBuilder::DoReturn(HReturn* instr) {
   return new(zone()) LReturn(UseFixed(instr->value(), rax));
 }

diff --git a/test/mjsunit/compiler/regress-toint32.js b/test/mjsunit/compiler/regress-toint32.js
new file mode 100644 (file)
index 0000000..54c2f76
--- /dev/null
+++ b/test/mjsunit/compiler/regress-toint32.js
@@ -0,0 +1,45 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax --noenable-sse3
+
+var a = new Int32Array(1);
+var G = 0x80000000;
+
+function f(x) {
+  var v = x;
+  v = v + 1;
+  a[0] = v;
+  v = v - 1;
+  return v;
+}
+
+assertEquals(G, f(G));
+assertEquals(G, f(G));
+%OptimizeFunctionOnNextCall(f);
+assertEquals(G, f(G));
+