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.