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 dc312e08958c6406698cc2493b6469eee2f326ff..3c177a9b8ca0a3dbcc2830c82148e45253d0d503 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -423,6 +423,21 @@ class HType {
     return IsHeapNumber() || IsString() || IsBoolean() || IsNonPrimitive();
   }
 
+  bool ToStringOrToNumberCanBeObserved(Representation representation) {
+    switch (type_) {

[Sven Panne, 2013/07/29 11:48:08]

The usual pattern within v8 is:

   switch (foo) {
     case BLAH:
     case BLOH:
       return 42;
     case FOO:
       return 271828;
   }
   UNREACHABLE();
   return 0;  // Make -Wall happy

I'll leave it up to you if you change it or not.

[Michael Starzinger, 2013/07/29 12:30:03]

+1 on this comment.

+    case kTaggedPrimitive:  // fallthru

[Sven Panne, 2013/07/29 11:48:08]

Align.

+      case kTaggedNumber:   // fallthru
+      case kSmi:            // fallthru
+      case kHeapNumber:     // fallthru
+      case kString:         // fallthru
+      case kBoolean: return false;
+      case kJSArray:        // fallthru
+      case kJSObject: return true;
+      case kTagged: break;
+    }
+    return !representation.IsSmiOrInteger32() && !representation.IsDouble();
+  }
+
   static HType TypeFromValue(Handle<Object> value);
 
   const char* ToString();
@@ -1126,6 +1141,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().ToStringOrToNumberCanBeObserved(representation());
+  }
+
+  // Returns true conservatively if the program might be able to observe a
+  // ToNumber() operation on this value.
+  bool ToNumberCanBeObserved() const {
+    return type().ToStringOrToNumberCanBeObserved(representation());
+  }
+
  protected:
   void TryGuaranteeRangeRecursive(RangeEvaluationContext* context);
 
@@ -3320,9 +3347,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) {
@@ -3342,6 +3366,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(); }
 };
 
 
@@ -3653,9 +3680,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() {
@@ -3909,9 +3936,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,
@@ -3948,6 +3972,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;
+  }
 };
 
 
@@ -6449,8 +6478,9 @@ 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; }
+  // No side-effects except possible allocation.
+  // NOTE: this instruction _does not_ call ToString() on its inputs.
+  virtual bool IsDeletable() const { return true; }
 
   const StringAddFlags flags_;
 };
@@ -6475,9 +6505,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)
 
@@ -6488,9 +6518,9 @@ 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:
+  // No side effects: runtime function assumes string + number inputs.
+  virtual bool IsDeletable() const { return true; }
 };
 
 
@@ -6505,10 +6535,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; }
 
@@ -6523,8 +6553,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();
+  }
 };