Reviewers: Hannes Payer,
Message:
Hannes: PTAL
Andreas, Bill: FYI
Description:
[crankshaft] DCE must not eliminate (observable) math operations.
The HUnaryMathOperation cannot be eliminated in general, because the
spec requires a ToNumber conversion on the input, which is observable
of course.
BUG=v8:4389
LOG=y
Please review this at https://codereview.chromium.org/1307413003/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+36, -55 lines):
M src/hydrogen-instructions.h
A + test/mjsunit/compiler/regress-4389-1.js
A + test/mjsunit/compiler/regress-4389-2.js
A + test/mjsunit/compiler/regress-4389-3.js
A + test/mjsunit/compiler/regress-4389-4.js
A + test/mjsunit/compiler/regress-4389-5.js
A + test/mjsunit/compiler/regress-4389-6.js
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
807a65102940649f0cd132a30b2cef254810323f..3264f40b21507e9d2344fdf020d96f2fe3c8268d
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -2635,7 +2635,12 @@ class HUnaryMathOperation final : public
HTemplateInstruction<2> {
SetFlag(kAllowUndefinedAsNaN);
}
- bool IsDeletable() const override { return true; }
+ bool IsDeletable() const override {
+ // TODO(crankshaft): This should be true, however the semantics of this
+ // instruction also include the ToNumber conversion that is mentioned
in the
+ // spec, which is of course observable.
+ return false;
+ }
HValue* SimplifiedDividendForMathFloorOfDiv(HDiv* hdiv);
HValue* SimplifiedDivisorForMathFloorOfDiv(HDiv* hdiv);
Index: test/mjsunit/compiler/regress-4389-1.js
diff --git a/test/mjsunit/compiler/regress-491578.js
b/test/mjsunit/compiler/regress-4389-1.js
similarity index 55%
copy from test/mjsunit/compiler/regress-491578.js
copy to test/mjsunit/compiler/regress-4389-1.js
index
c27570456c31e9a191fe1f4f1ea17b80f29de98e..c58ce2da401ef1c279c8b706d4cf45911a317cd9
100644
--- a/test/mjsunit/compiler/regress-491578.js
+++ b/test/mjsunit/compiler/regress-4389-1.js
@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --dead-code-elimination
-function foo(x) {
- if (x === undefined) return;
- while (true) {
- while (1 || 2) { }
- f();
- }
-}
+function foo(x) { Math.fround(x); }
+foo(1);
+foo(2);
%OptimizeFunctionOnNextCall(foo);
-foo();
+assertThrows(function() { foo(Symbol()) }, TypeError);
Index: test/mjsunit/compiler/regress-4389-2.js
diff --git a/test/mjsunit/compiler/regress-491578.js
b/test/mjsunit/compiler/regress-4389-2.js
similarity index 55%
copy from test/mjsunit/compiler/regress-491578.js
copy to test/mjsunit/compiler/regress-4389-2.js
index
c27570456c31e9a191fe1f4f1ea17b80f29de98e..3b720a5a9541f990130cb85e118b8e55b64350a2
100644
--- a/test/mjsunit/compiler/regress-491578.js
+++ b/test/mjsunit/compiler/regress-4389-2.js
@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --dead-code-elimination
-function foo(x) {
- if (x === undefined) return;
- while (true) {
- while (1 || 2) { }
- f();
- }
-}
+function foo(x) { Math.sqrt(x); }
+foo(1);
+foo(2);
%OptimizeFunctionOnNextCall(foo);
-foo();
+assertThrows(function() { foo(Symbol()) }, TypeError);
Index: test/mjsunit/compiler/regress-4389-3.js
diff --git a/test/mjsunit/compiler/regress-491578.js
b/test/mjsunit/compiler/regress-4389-3.js
similarity index 55%
copy from test/mjsunit/compiler/regress-491578.js
copy to test/mjsunit/compiler/regress-4389-3.js
index
c27570456c31e9a191fe1f4f1ea17b80f29de98e..9aa72d1ac9ff196c5c113c207c7a1e414f84f950
100644
--- a/test/mjsunit/compiler/regress-491578.js
+++ b/test/mjsunit/compiler/regress-4389-3.js
@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --dead-code-elimination
-function foo(x) {
- if (x === undefined) return;
- while (true) {
- while (1 || 2) { }
- f();
- }
-}
+function foo(x) { Math.floor(x); }
+foo(1);
+foo(2);
%OptimizeFunctionOnNextCall(foo);
-foo();
+assertThrows(function() { foo(Symbol()) }, TypeError);
Index: test/mjsunit/compiler/regress-4389-4.js
diff --git a/test/mjsunit/compiler/regress-491578.js
b/test/mjsunit/compiler/regress-4389-4.js
similarity index 55%
copy from test/mjsunit/compiler/regress-491578.js
copy to test/mjsunit/compiler/regress-4389-4.js
index
c27570456c31e9a191fe1f4f1ea17b80f29de98e..e824973fac43efa3631b799f2fe3b93df8f93e29
100644
--- a/test/mjsunit/compiler/regress-491578.js
+++ b/test/mjsunit/compiler/regress-4389-4.js
@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --dead-code-elimination
-function foo(x) {
- if (x === undefined) return;
- while (true) {
- while (1 || 2) { }
- f();
- }
-}
+function foo(x) { Math.round(x); }
+foo(1);
+foo(2);
%OptimizeFunctionOnNextCall(foo);
-foo();
+assertThrows(function() { foo(Symbol()) }, TypeError);
Index: test/mjsunit/compiler/regress-4389-5.js
diff --git a/test/mjsunit/compiler/regress-491578.js
b/test/mjsunit/compiler/regress-4389-5.js
similarity index 55%
copy from test/mjsunit/compiler/regress-491578.js
copy to test/mjsunit/compiler/regress-4389-5.js
index
c27570456c31e9a191fe1f4f1ea17b80f29de98e..64797bc76c33e05d33afb7aca0ccb97b05c3e42b
100644
--- a/test/mjsunit/compiler/regress-491578.js
+++ b/test/mjsunit/compiler/regress-4389-5.js
@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --dead-code-elimination
-function foo(x) {
- if (x === undefined) return;
- while (true) {
- while (1 || 2) { }
- f();
- }
-}
+function foo(x) { Math.abs(x); }
+foo(1);
+foo(2);
%OptimizeFunctionOnNextCall(foo);
-foo();
+assertThrows(function() { foo(Symbol()) }, TypeError);
Index: test/mjsunit/compiler/regress-4389-6.js
diff --git a/test/mjsunit/compiler/regress-491578.js
b/test/mjsunit/compiler/regress-4389-6.js
similarity index 55%
copy from test/mjsunit/compiler/regress-491578.js
copy to test/mjsunit/compiler/regress-4389-6.js
index
c27570456c31e9a191fe1f4f1ea17b80f29de98e..fe065707f4231b8cfa6bf67edc30997c3c342957
100644
--- a/test/mjsunit/compiler/regress-491578.js
+++ b/test/mjsunit/compiler/regress-4389-6.js
@@ -2,14 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --dead-code-elimination
-function foo(x) {
- if (x === undefined) return;
- while (true) {
- while (1 || 2) { }
- f();
- }
-}
+function foo(x) { Math.log(x); }
+foo(1);
+foo(2);
%OptimizeFunctionOnNextCall(foo);
-foo();
+assertThrows(function() { foo(Symbol()) }, TypeError);
--
--
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/d/optout.
