Fix IsDeletable() for HStringAdd, HStringCharCodeAt, HStringCharFromCode.

src/hydrogen-instructions.h

Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index b43efc80baa9cb111b764a375e656ab9f522538e..8fa700f4bc022050b5e7930b41326f196a275bda 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -1142,6 +1142,26 @@ class HValue: public ZoneObject {
     }
   }
 
+  // Returns true conservatively if the program might be able to observe a
+  // ToString() operation on this value.
+  bool ToStringCanBeObserved() const {
+    if (type().IsUninitialized()) {
+      // If the type is not known, use the representation as a hint.
+      return representation().IsSmiOrInteger32() || representation().IsDouble();

[Michael Starzinger, 2013/07/26 08:03:03]

Discusssed with Sven: This whole line should be negated.

+    }
+    return !type().IsNonPrimitive();

[Michael Starzinger, 2013/07/26 08:03:03]

Discusssed with Sven: What happens for type().IsTagged() here?

+  }
+
+  // Returns true conservatively if the program might be able to observe a
+  // ToNumber() operation on this value.
+  bool ToNumberCanBeObserved() const {
+    if (type().IsUninitialized()) {
+      // If the type is not known, use the representation as a hint.
+      return representation().IsSmiOrInteger32() || representation().IsDouble();

[Michael Starzinger, 2013/07/26 08:03:03]

Likewise.

+    }
+    return !type().IsNonPrimitive();

[Michael Starzinger, 2013/07/26 08:03:03]

Likewise.

+  }
+
  protected:
   void TryGuaranteeRangeRecursive(RangeEvaluationContext* context);
 
@@ -3336,9 +3356,6 @@ class HPhi: public HValue {
 
   void SimplifyConstantInputs();
 
-  // TODO(titzer): we can't eliminate the receiver for generating backtraces
-  virtual bool IsDeletable() const { return !IsReceiver(); }
-
  protected:
   virtual void DeleteFromGraph();
   virtual void InternalSetOperandAt(int index, HValue* value) {
@@ -3358,6 +3375,9 @@ class HPhi: public HValue {
   int indirect_uses_[Representation::kNumRepresentations];
   int phi_id_;
   InductionVariableData* induction_variable_data_;
+
+  // TODO(titzer): we can't eliminate the receiver for generating backtraces
+  virtual bool IsDeletable() const { return !IsReceiver(); }
 };
 
 
@@ -3671,9 +3691,9 @@ class HBinaryOperation: public HTemplateInstruction<3> {
     observed_input_representation_[1] = Representation::None();
   }
 
-  HValue* context() { return OperandAt(0); }
-  HValue* left() { return OperandAt(1); }
-  HValue* right() { return OperandAt(2); }
+  HValue* context() const { return OperandAt(0); }
+  HValue* left() const { return OperandAt(1); }
+  HValue* right() const { return OperandAt(2); }
 
   // True if switching left and right operands likely generates better code.
   bool AreOperandsBetterSwitched() {
@@ -3923,9 +3943,6 @@ class HBoundsCheck: public HTemplateInstruction<2> {
   virtual Representation RequiredInputRepresentation(int arg_index) {
     return representation();
   }
-  virtual bool IsDeletable() const {
-    return skip_check() && !FLAG_debug_code;
-  }
 
   virtual bool IsRelationTrueInternal(NumericRelation relation,
                                       HValue* related_value,
@@ -3962,6 +3979,11 @@ class HBoundsCheck: public HTemplateInstruction<2> {
   int scale_;
   RangeGuaranteeDirection responsibility_direction_;
   bool allow_equality_;
+
+ private:
+  virtual bool IsDeletable() const {
+    return skip_check() && !FLAG_debug_code;
+  }
 };
 
 
@@ -6396,8 +6418,11 @@ class HStringAdd: public HBinaryOperation {
     SetGVNFlag(kChangesNewSpacePromotion);
   }
 
-  // TODO(svenpanne) Might be safe, but leave it out until we know for sure.
-  //  virtual bool IsDeletable() const { return true; }
+  virtual bool IsDeletable() const {
+    if (flags_ == STRING_ADD_CHECK_NONE) return true;
+    return !left()->ToStringCanBeObserved()
+        && !right()->ToStringCanBeObserved();
+  }
 
   const StringAddFlags flags_;
 };
@@ -6422,9 +6447,9 @@ class HStringCharCodeAt: public HTemplateInstruction<3> {
         : Representation::Tagged();
   }
 
-  HValue* context() { return OperandAt(0); }
-  HValue* string() { return OperandAt(1); }
-  HValue* index() { return OperandAt(2); }
+  HValue* context() const { return OperandAt(0); }
+  HValue* string() const { return OperandAt(1); }
+  HValue* index() const { return OperandAt(2); }
 
   DECLARE_CONCRETE_INSTRUCTION(StringCharCodeAt)
 
@@ -6435,9 +6460,11 @@ class HStringCharCodeAt: public HTemplateInstruction<3> {
     return new(zone) Range(0, String::kMaxUtf16CodeUnit);
   }
 
-  // TODO(svenpanne) Might be safe, but leave it out until we know for sure.
-  // private:
-  //  virtual bool IsDeletable() const { return true; }
+ private:
+  virtual bool IsDeletable() const {
+    // NOTE: we assume this instruction is dominated by a string typecheck.
+    return !index()->ToNumberCanBeObserved();
+  }
 };
 
 
@@ -6452,10 +6479,10 @@ class HStringCharFromCode: public HTemplateInstruction<2> {
         ? Representation::Tagged()
         : Representation::Integer32();
   }
-  virtual HType CalculateInferredType();
+  virtual HType CalculateInferredType() { return HType::String(); }
 
-  HValue* context() { return OperandAt(0); }
-  HValue* value() { return OperandAt(1); }
+  HValue* context() const { return OperandAt(0); }
+  HValue* value() const { return OperandAt(1); }
 
   virtual bool DataEquals(HValue* other) { return true; }
 
@@ -6470,8 +6497,9 @@ class HStringCharFromCode: public HTemplateInstruction<2> {
     SetGVNFlag(kChangesNewSpacePromotion);
   }
 
-  // TODO(svenpanne) Might be safe, but leave it out until we know for sure.
-  // virtual bool IsDeletable() const { return true; }
+  virtual bool IsDeletable() const {
+    return !value()->ToNumberCanBeObserved();
+  }
 };