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..dc657645e3d8dfd1791ccce741403a8450845862 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -1142,6 +1142,18 @@ class HValue: public ZoneObject {
     }
   }
 
+  // Returns true conservatively if the program might be able to observe a
+  // ToString() operation on this value.
+  bool ToStringCanBeObserved() const {
+    return !type().IsString() && representation().IsTagged();
+  }
+
+  // Returns true conservatively if the program might be able to observe a
+  // ToNumber() operation on this value.
+  bool ToNumberCanBeObserved() const {
+    return !type().IsString() && representation().IsTagged();

[Michael Starzinger, 2013/07/25 14:43:18]

This predicate looks broken. Shouldn't that be type().IsNumber() here?

+  }
+
  protected:
   void TryGuaranteeRangeRecursive(RangeEvaluationContext* context);
 
@@ -3671,9 +3683,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() {
@@ -6396,8 +6408,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 +6437,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 +6450,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,15 +6469,19 @@ 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; }
 
   DECLARE_CONCRETE_INSTRUCTION(StringCharFromCode)
 
+  virtual bool IsDeletable() const {

[Michael Starzinger, 2013/07/25 14:43:18]

nit: This should be private, please move it back down.

[titzer, 2013/07/25 15:32:55]

Done.

+    return !value()->ToNumberCanBeObserved();
+  }
+
  private:
   HStringCharFromCode(HValue* context, HValue* char_code) {
     SetOperandAt(0, context);
@@ -6469,9 +6490,6 @@ class HStringCharFromCode: public HTemplateInstruction<2> {
     SetFlag(kUseGVN);
     SetGVNFlag(kChangesNewSpacePromotion);
   }
-
-  // TODO(svenpanne) Might be safe, but leave it out until we know for sure.
-  // virtual bool IsDeletable() const { return true; }
 };