[v8-dev] Fix IsDeletable() for HStringAdd, HStringCharCodeAt, HStringCharFromCode. (issue 20241005)

Thu, 25 Jul 2013 07:36:29 -0700 

Reviewers: Michael Starzinger, Sven Panne,

Description:
Fix IsDeletable() for HStringAdd, HStringCharCodeAt, HStringCharFromCode.

BUG=

Please review this at https://codereview.chromium.org/20241005/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/hydrogen-instructions.h
  M src/hydrogen-instructions.cc
  M src/hydrogen.cc


Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index 707e81842f8364ac000a742f936b2b6b38838762..6210a705df71ebaa6519f2d907e94d4facd8bc72 100644 
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -3685,11 +3685,6 @@ Representation HUnaryMathOperation::RepresentationFromInputs() { 
 }


-HType HStringCharFromCode::CalculateInferredType() {
-  return HType::String();
-}
-
-
 void HAllocate::HandleSideEffectDominator(GVNFlag side_effect,
                                           HValue* dominator) {
   ASSERT(side_effect == kChangesNewSpacePromotion);
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();
+  }
+
  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 {
+    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; }
 };


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 0aae86129732f4b93ff35306dddcc38b05056e9a..03f7eb447690ee594a9e6a0c55a9104a1809a015 100644 
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -9078,7 +9078,6 @@ void HOptimizedGraphBuilder::GenerateRandomHeapNumber(CallRuntime* call) { 
 }


-// Fast support for StringAdd.
 void HOptimizedGraphBuilder::GenerateStringAdd(CallRuntime* call) {
   ASSERT_EQ(2, call->arguments()->length());
   CHECK_ALIVE(VisitForValue(call->arguments()->at(0)));


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to