Revision: 17147
Author:   [email protected]
Date:     Thu Oct 10 16:49:25 2013 UTC
Log:      Only set binary operation side effects flags, when observable.

BUG=
[email protected]

Review URL: https://codereview.chromium.org/26712002
http://code.google.com/p/v8/source/detail?r=17147

Modified:
 /branches/bleeding_edge/src/hydrogen-instructions.h
 /branches/bleeding_edge/test/mjsunit/regress/regress-binop.js

=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Fri Oct 4 19:04:34 2013 UTC +++ /branches/bleeding_edge/src/hydrogen-instructions.h Thu Oct 10 16:49:25 2013 UTC
@@ -3947,11 +3947,12 @@
   }

   virtual void RepresentationChanged(Representation to) V8_OVERRIDE {
-    if (to.IsTagged()) {
+    if (to.IsTagged()) SetGVNFlag(kChangesNewSpacePromotion);
+    if (to.IsTagged() &&
+ (left()->ToNumberCanBeObserved() || right()->ToNumberCanBeObserved())) {
       SetAllSideEffects();
       ClearFlag(kUseGVN);
     } else {
-      ASSERT(to.IsSmiOrInteger32());
       ClearAllSideEffects();
       SetFlag(kUseGVN);
     }
@@ -4023,7 +4024,9 @@
   }

   virtual void RepresentationChanged(Representation to) V8_OVERRIDE {
-    if (to.IsTagged()) {
+    if (to.IsTagged()) SetGVNFlag(kChangesNewSpacePromotion);
+    if (to.IsTagged() &&
+ (left()->ToNumberCanBeObserved() || right()->ToNumberCanBeObserved())) {
       SetAllSideEffects();
       ClearFlag(kUseGVN);
     } else {
@@ -4562,8 +4565,19 @@
   }

   virtual void RepresentationChanged(Representation to) V8_OVERRIDE {
-    if (to.IsTagged()) ClearFlag(kAllowUndefinedAsNaN);
-    HArithmeticBinaryOperation::RepresentationChanged(to);
+    if (to.IsTagged()) {
+      SetGVNFlag(kChangesNewSpacePromotion);
+      ClearFlag(kAllowUndefinedAsNaN);
+    }
+    if (to.IsTagged() &&
+ (left()->ToNumberCanBeObserved() || right()->ToNumberCanBeObserved() || + left()->ToStringCanBeObserved() || right()->ToStringCanBeObserved())) {
+      SetAllSideEffects();
+      ClearFlag(kUseGVN);
+    } else {
+      ClearAllSideEffects();
+      SetFlag(kUseGVN);
+    }
   }

   DECLARE_CONCRETE_INSTRUCTION(Add)
@@ -6632,20 +6646,25 @@
HStringAdd(HValue* context, HValue* left, HValue* right, StringAddFlags flags) : HBinaryOperation(context, left, right, HType::String()), flags_(flags) {
     set_representation(Representation::Tagged());
-    if (flags_ == STRING_ADD_CHECK_NONE) {
+    if (MightHaveSideEffects()) {
+      SetAllSideEffects();
+    } else {
       SetFlag(kUseGVN);
       SetGVNFlag(kDependsOnMaps);
       SetGVNFlag(kChangesNewSpacePromotion);
-    } else {
-      SetAllSideEffects();
     }
   }
+
+  bool MightHaveSideEffects() const {
+    return flags_ != STRING_ADD_CHECK_NONE &&
+ (left()->ToStringCanBeObserved() || right()->ToStringCanBeObserved());
+  }

   // No side-effects except possible allocation:
// NOTE: this instruction does not call ToString() on its inputs, when flags_
   // is set to STRING_ADD_CHECK_NONE.
   virtual bool IsDeletable() const V8_OVERRIDE {
-    return flags_ == STRING_ADD_CHECK_NONE;
+    return !MightHaveSideEffects();
   }

   const StringAddFlags flags_;
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-binop.js Fri Sep 20 08:34:23 2013 UTC +++ /branches/bleeding_edge/test/mjsunit/regress/regress-binop.js Thu Oct 10 16:49:25 2013 UTC
@@ -166,3 +166,16 @@
 assertEquals(t2(1,1<<30), 1/(1<<30));
 assertEquals(t2(1,2), 1/2);

+
+// Assert that the hole is not truncated to nan for string add.
+function string_add(a,i) {
+  var d = [0.1, ,0.3];
+  return a + d[i];
+}
+
+string_add(1.1, 0);
+string_add("", 0);
+%OptimizeFunctionOnNextCall(string_add);
+string_add(1.1, 0);
+// There comes the hole
+assertEquals("undefined", string_add("", 1));

--
--
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