Fix IsDeletable() for HStringAdd, HStringCharCodeAt, HStringCharFromCode.

src/hydrogen-instructions.h

 // 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
@@ skipping 1124 matching lines @@
 
   bool TryGuaranteeRange(HValue* upper_bound);
   virtual bool TryDecompose(DecompositionResult* decomposition) {
     if (RedefinedOperand() != NULL) {
       return RedefinedOperand()->TryDecompose(decomposition);
     } else {
       return false;
     }
   }
 
+  // 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);
 
   enum RangeGuaranteeDirection {
     DIRECTION_NONE = 0,
     DIRECTION_UPPER = 1,
     DIRECTION_LOWER = 2,
     DIRECTION_BOTH = DIRECTION_UPPER | DIRECTION_LOWER
   };
   virtual void SetResponsibilityForRange(RangeGuaranteeDirection direction) {}
@@ skipping 2509 matching lines @@
   HBinaryOperation(HValue* context, HValue* left, HValue* right)
       : observed_output_representation_(Representation::None()) {
     ASSERT(left != NULL && right != NULL);
     SetOperandAt(0, context);
     SetOperandAt(1, left);
     SetOperandAt(2, right);
     observed_input_representation_[0] = Representation::None();
     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() {
     if (!IsCommutative()) return false;
 
     // Constant operands are better off on the right, they can be inlined in
     // many situations on most platforms.
     if (left()->IsConstant()) return true;
     if (right()->IsConstant()) return false;
 
@@ skipping 2702 matching lines @@
 
  private:
   HStringAdd(HValue* context, HValue* left, HValue* right, StringAddFlags flags)
       : HBinaryOperation(context, left, right), flags_(flags) {
     set_representation(Representation::Tagged());
     SetFlag(kUseGVN);
     SetGVNFlag(kDependsOnMaps);
     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_;
 };
 
 
 class HStringCharCodeAt: public HTemplateInstruction<3> {
  public:
   HStringCharCodeAt(HValue* context, HValue* string, HValue* index) {
     SetOperandAt(0, context);
     SetOperandAt(1, string);
     SetOperandAt(2, index);
     set_representation(Representation::Integer32());
     SetFlag(kUseGVN);
     SetGVNFlag(kDependsOnMaps);
     SetGVNFlag(kChangesNewSpacePromotion);
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
     // The index is supposed to be Integer32.
     return index == 2
         ? Representation::Integer32()
         : 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)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
   virtual Range* InferRange(Zone* zone) {
     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();
+  }
 };
 
 
 class HStringCharFromCode: public HTemplateInstruction<2> {
  public:
   static HInstruction* New(Zone* zone,
                            HValue* context,
                            HValue* char_code);
 
   virtual Representation RequiredInputRepresentation(int index) {
     return index == 0
         ? 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);
     SetOperandAt(1, char_code);
     set_representation(Representation::Tagged());
     SetFlag(kUseGVN);
     SetGVNFlag(kChangesNewSpacePromotion);
   }
-
-  // TODO(svenpanne) Might be safe, but leave it out until we know for sure.
-  // virtual bool IsDeletable() const { return true; }
 };
 
 
 class HStringLength: public HUnaryOperation {
  public:
   static HInstruction* New(Zone* zone, HValue* string);
 
   virtual Representation RequiredInputRepresentation(int index) {
     return Representation::Tagged();
   }
@@ skipping 388 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_