Fix the hole loading optimization.

- Holes are only ever loaded as double or tagged.
- Change to tagged has to deoptimize on undefined (no implicit
  conversions from double the hole NaN -> tagged undefined).

BUG=
R=jkummerow@chromium.org

Review URL: https://chromiumcodereview.appspot.com/16099006

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

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index aed6492..d15fe7a 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -4963,7 +4963,9 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
 
   Label load_smi, heap_number, done;
 
-  if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) {
+  STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE >
+                NUMBER_CANDIDATE_IS_ANY_TAGGED);
+  if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) {
     // Smi check.
     __ UntagAndJumpIfSmi(scratch, input_reg, &load_smi);
 
@@ -4974,14 +4976,20 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
     if (deoptimize_on_undefined) {
       DeoptimizeIf(ne, env);
     } else {
-      Label heap_number;
+      Label heap_number, convert;
       __ b(eq, &heap_number);
 
+      // Convert undefined (and hole) to NaN.
       __ LoadRoot(ip, Heap::kUndefinedValueRootIndex);
       __ cmp(input_reg, Operand(ip));
+      if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) {
+        __ b(eq, &convert);
+        __ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
+        __ cmp(input_reg, Operand(ip));
+      }
       DeoptimizeIf(ne, env);
 
-      // Convert undefined to NaN.
+      __ bind(&convert);
       __ LoadRoot(ip, Heap::kNanValueRootIndex);
       __ sub(ip, ip, Operand(kHeapObjectTag));
       __ vldr(result_reg, ip, HeapNumber::kValueOffset);
@@ -5001,15 +5009,6 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
       DeoptimizeIf(eq, env);
     }
     __ jmp(&done);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) {
-    __ SmiUntag(scratch, input_reg, SetCC);
-    DeoptimizeIf(cs, env);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) {
-    __ UntagAndJumpIfSmi(scratch, input_reg, &load_smi);
-    __ Vmov(result_reg,
-            FixedDoubleArray::hole_nan_as_double(),
-            no_reg);
-    __ b(&done);
   } else {
     __ SmiUntag(scratch, input_reg);
     ASSERT(mode == NUMBER_CANDIDATE_IS_SMI);
@@ -5133,19 +5132,13 @@ void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) {
   NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED;
   HValue* value = instr->hydrogen()->value();
   if (value->type().IsSmi()) {
-    if (value->IsLoadKeyed()) {
-      HLoadKeyed* load = HLoadKeyed::cast(value);
-      if (load->UsesMustHandleHole()) {
-        if (load->hole_mode() == ALLOW_RETURN_HOLE) {
-          mode = NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE;
-        } else {
-          mode = NUMBER_CANDIDATE_IS_SMI_OR_HOLE;
-        }
-      } else {
-        mode = NUMBER_CANDIDATE_IS_SMI;
+    mode = NUMBER_CANDIDATE_IS_SMI;
+  } else if (value->IsLoadKeyed()) {
+    HLoadKeyed* load = HLoadKeyed::cast(value);
+    if (load->UsesMustHandleHole()) {
+      if (load->hole_mode() == ALLOW_RETURN_HOLE) {
+        mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE;
       }
-    } else {
-      mode = NUMBER_CANDIDATE_IS_SMI;
     }
   }
 

diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index f1346d5..e7e63d1 100644 (file)
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -2720,11 +2720,14 @@ bool HLoadKeyed::UsesMustHandleHole() const {
     return false;
   }
 
+  // Holes are only returned as tagged values.
+  if (!representation().IsTagged()) {
+    return false;
+  }
+
   for (HUseIterator it(uses()); !it.Done(); it.Advance()) {
     HValue* use = it.value();
-    if (!use->IsChange() || !HChange::cast(use)->to().IsDouble()) {
-      return false;
-    }
+    if (!use->IsChange()) return false;
   }
 
   return true;

diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 6fbfd15..5d4b90a 100644 (file)
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -1718,7 +1718,9 @@ class HChange: public HUnaryOperation {
     ASSERT(!value->representation().Equals(to));
     set_representation(to);
     SetFlag(kUseGVN);
-    if (deoptimize_on_undefined) SetFlag(kDeoptimizeOnUndefined);
+    if (deoptimize_on_undefined || to.IsTagged()) {
+      SetFlag(kDeoptimizeOnUndefined);
+    }
     if (is_truncating) SetFlag(kTruncatingToInt32);
     if (value->representation().IsSmi() || value->type().IsSmi()) {
       set_type(HType::Smi());

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index b3d12c0..b473f58 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -3988,8 +3988,8 @@ bool HGraph::Optimize(SmartArrayPointer<char>* bailout_reason) {
   // This must happen after inferring representations.
   MergeRemovableSimulates();
 
-  MarkDeoptimizeOnUndefined();
   InsertRepresentationChanges();
+  MarkDeoptimizeOnUndefined();
 
   InitializeInferredTypes();
 

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 4a0895f..dddd1c6 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -5076,7 +5076,9 @@ void LCodeGen::EmitNumberUntagDNoSSE2(Register input_reg,
                                       NumberUntagDMode mode) {
   Label load_smi, done;
 
-  if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) {
+  STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE >
+                NUMBER_CANDIDATE_IS_ANY_TAGGED);
+  if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) {
     // Smi check.
     __ JumpIfSmi(input_reg, &load_smi, Label::kNear);
 
@@ -5086,17 +5088,23 @@ void LCodeGen::EmitNumberUntagDNoSSE2(Register input_reg,
     if (deoptimize_on_undefined) {
       DeoptimizeIf(not_equal, env);
     } else {
-      Label heap_number;
+      Label heap_number, convert;
       __ j(equal, &heap_number, Label::kNear);
 
+      // Convert undefined (or hole) to NaN.
       __ cmp(input_reg, factory()->undefined_value());
+      if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) {
+        __ j(equal, &convert, Label::kNear);
+        __ cmp(input_reg, factory()->the_hole_value());
+      }
       DeoptimizeIf(not_equal, env);
 
-      // Convert undefined to NaN.
+      __ bind(&convert);
       ExternalReference nan =
           ExternalReference::address_of_canonical_non_hole_nan();
       __ fld_d(Operand::StaticVariable(nan));
       __ jmp(&done, Label::kNear);
+
       __ bind(&heap_number);
     }
     // Heap number to x87 conversion.
@@ -5117,16 +5125,6 @@ void LCodeGen::EmitNumberUntagDNoSSE2(Register input_reg,
       DeoptimizeIf(not_zero, env);
     }
     __ jmp(&done, Label::kNear);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) {
-    __ test(input_reg, Immediate(kSmiTagMask));
-    DeoptimizeIf(not_equal, env);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) {
-    __ test(input_reg, Immediate(kSmiTagMask));
-    __ j(zero, &load_smi);
-    ExternalReference hole_nan_reference =
-        ExternalReference::address_of_the_hole_nan();
-    __ fld_d(Operand::StaticVariable(hole_nan_reference));
-    __ jmp(&done, Label::kNear);
   } else {
     ASSERT(mode == NUMBER_CANDIDATE_IS_SMI);
   }
@@ -5150,7 +5148,9 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
                                 NumberUntagDMode mode) {
   Label load_smi, done;
 
-  if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) {
+  STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE >
+                NUMBER_CANDIDATE_IS_ANY_TAGGED);
+  if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) {
     // Smi check.
     __ JumpIfSmi(input_reg, &load_smi, Label::kNear);
 
@@ -5160,13 +5160,18 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
     if (deoptimize_on_undefined) {
       DeoptimizeIf(not_equal, env);
     } else {
-      Label heap_number;
+      Label heap_number, convert;
       __ j(equal, &heap_number, Label::kNear);
 
+      // Convert undefined (and hole) to NaN.
       __ cmp(input_reg, factory()->undefined_value());
+      if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) {
+        __ j(equal, &convert, Label::kNear);
+        __ cmp(input_reg, factory()->the_hole_value());
+      }
       DeoptimizeIf(not_equal, env);
 
-      // Convert undefined to NaN.
+      __ bind(&convert);
       ExternalReference nan =
           ExternalReference::address_of_canonical_non_hole_nan();
       __ movdbl(result_reg, Operand::StaticVariable(nan));
@@ -5186,16 +5191,6 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
       DeoptimizeIf(not_zero, env);
     }
     __ jmp(&done, Label::kNear);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) {
-    __ test(input_reg, Immediate(kSmiTagMask));
-    DeoptimizeIf(not_equal, env);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) {
-    __ test(input_reg, Immediate(kSmiTagMask));
-    __ j(zero, &load_smi);
-    ExternalReference hole_nan_reference =
-        ExternalReference::address_of_the_hole_nan();
-    __ movdbl(result_reg, Operand::StaticVariable(hole_nan_reference));
-    __ jmp(&done, Label::kNear);
   } else {
     ASSERT(mode == NUMBER_CANDIDATE_IS_SMI);
   }
@@ -5495,20 +5490,12 @@ void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) {
 
   NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED;
   HValue* value = instr->hydrogen()->value();
-  if (value->type().IsSmi()) {
-    if (value->IsLoadKeyed()) {
-      HLoadKeyed* load = HLoadKeyed::cast(value);
-      if (load->UsesMustHandleHole()) {
-        if (load->hole_mode() == ALLOW_RETURN_HOLE) {
-          mode = NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE;
-        } else {
-          mode = NUMBER_CANDIDATE_IS_SMI_OR_HOLE;
-        }
-      } else {
-        mode = NUMBER_CANDIDATE_IS_SMI;
-      }
-    } else {
-      mode = NUMBER_CANDIDATE_IS_SMI;
+  if (value->representation().IsSmi()) {
+    mode = NUMBER_CANDIDATE_IS_SMI;
+  } else if (value->IsLoadKeyed()) {
+    HLoadKeyed* load = HLoadKeyed::cast(value);
+    if (load->UsesMustHandleHole()) {
+      mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE;
     }
   }
 

diff --git a/src/lithium.h b/src/lithium.h
index 85df95a..170e5c8 100644 (file)
--- a/src/lithium.h
+++ b/src/lithium.h
@@ -769,9 +769,8 @@ int StackSlotOffset(int index);
 
 enum NumberUntagDMode {
   NUMBER_CANDIDATE_IS_SMI,
-  NUMBER_CANDIDATE_IS_SMI_OR_HOLE,
-  NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE,
-  NUMBER_CANDIDATE_IS_ANY_TAGGED
+  NUMBER_CANDIDATE_IS_ANY_TAGGED,
+  NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE
 };
 
 

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index f9b6e81..b42e660 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -4663,7 +4663,9 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
                                 NumberUntagDMode mode) {
   Label load_smi, done;
 
-  if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED) {
+  STATIC_ASSERT(NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE >
+                NUMBER_CANDIDATE_IS_ANY_TAGGED);
+  if (mode >= NUMBER_CANDIDATE_IS_ANY_TAGGED) {
     // Smi check.
     __ JumpIfSmi(input_reg, &load_smi, Label::kNear);
 
@@ -4673,13 +4675,18 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
     if (deoptimize_on_undefined) {
       DeoptimizeIf(not_equal, env);
     } else {
-      Label heap_number;
+      Label heap_number, convert;
       __ j(equal, &heap_number, Label::kNear);
 
+      // Convert undefined (and hole) to NaN. Compute NaN as 0/0.
       __ CompareRoot(input_reg, Heap::kUndefinedValueRootIndex);
+      if (mode == NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE) {
+        __ j(equal, &convert, Label::kNear);
+        __ CompareRoot(input_reg, Heap::kTheHoleValueRootIndex);
+      }
       DeoptimizeIf(not_equal, env);
 
-      // Convert undefined to NaN. Compute NaN as 0/0.
+      __ bind(&convert);
       __ xorps(result_reg, result_reg);
       __ divsd(result_reg, result_reg);
       __ jmp(&done, Label::kNear);
@@ -4698,16 +4705,6 @@ void LCodeGen::EmitNumberUntagD(Register input_reg,
       DeoptimizeIf(not_zero, env);
     }
     __ jmp(&done, Label::kNear);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_OR_HOLE) {
-      __ testq(input_reg, Immediate(kSmiTagMask));
-      DeoptimizeIf(not_equal, env);
-  } else if (mode == NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE) {
-      __ testq(input_reg, Immediate(kSmiTagMask));
-      __ j(zero, &load_smi);
-    __ Set(kScratchRegister, BitCast<uint64_t>(
-        FixedDoubleArray::hole_nan_as_double()));
-    __ movq(result_reg, kScratchRegister);
-      __ jmp(&done, Label::kNear);
   } else {
     ASSERT(mode == NUMBER_CANDIDATE_IS_SMI);
   }
@@ -4802,19 +4799,11 @@ void LCodeGen::DoNumberUntagD(LNumberUntagD* instr) {
   NumberUntagDMode mode = NUMBER_CANDIDATE_IS_ANY_TAGGED;
   HValue* value = instr->hydrogen()->value();
   if (value->type().IsSmi()) {
-    if (value->IsLoadKeyed()) {
-      HLoadKeyed* load = HLoadKeyed::cast(value);
-      if (load->UsesMustHandleHole()) {
-        if (load->hole_mode() == ALLOW_RETURN_HOLE) {
-          mode = NUMBER_CANDIDATE_IS_SMI_CONVERT_HOLE;
-        } else {
-          mode = NUMBER_CANDIDATE_IS_SMI_OR_HOLE;
-        }
-      } else {
-        mode = NUMBER_CANDIDATE_IS_SMI;
-      }
-    } else {
-      mode = NUMBER_CANDIDATE_IS_SMI;
+    mode = NUMBER_CANDIDATE_IS_SMI;
+  } else if (value->IsLoadKeyed()) {
+    HLoadKeyed* load = HLoadKeyed::cast(value);
+    if (load->UsesMustHandleHole()) {
+      mode = NUMBER_CANDIDATE_IS_ANY_TAGGED_CONVERT_HOLE;
     }
   }
 

diff --git a/test/mjsunit/regress/regress-convert-hole.js b/test/mjsunit/regress/regress-convert-hole.js
new file mode 100644 (file)
index 0000000..1424221
--- /dev/null
+++ b/test/mjsunit/regress/regress-convert-hole.js
@@ -0,0 +1,52 @@
+// Copyright 2013 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
+
+function f(test, test2, a, i) {
+  var o = [0.5,1,,3];
+  var d;
+  if (test) {
+    d = 1.5;
+  } else {
+    d = o[i];
+  }
+  if (test2) {
+    d += 1;
+  }
+  a[i] = d;
+  return d;
+}
+
+var a = [0, 0, 0, {}];
+f(true, false, a, 0);
+f(true, true, a, 0);
+f(false, false, a, 1);
+f(false, true, a, 1);
+%OptimizeFunctionOnNextCall(f);
+f(false, false, a, 2);
+assertEquals(undefined, a[2]);