From fef38c21e8e119e3bbf5423fd6fc2c03fd1f6c39 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Mon, 24 Aug 2015 23:24:40 -0700 Subject: [PATCH] [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 Review URL: https://codereview.chromium.org/1307413003 Cr-Commit-Position: refs/heads/master@{#30343} --- src/hydrogen-instructions.h | 7 ++++++- test/mjsunit/compiler/regress-4389-1.js | 11 +++++++++++ test/mjsunit/compiler/regress-4389-2.js | 11 +++++++++++ test/mjsunit/compiler/regress-4389-3.js | 11 +++++++++++ test/mjsunit/compiler/regress-4389-4.js | 11 +++++++++++ test/mjsunit/compiler/regress-4389-5.js | 11 +++++++++++ test/mjsunit/compiler/regress-4389-6.js | 11 +++++++++++ 7 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 test/mjsunit/compiler/regress-4389-1.js create mode 100644 test/mjsunit/compiler/regress-4389-2.js create mode 100644 test/mjsunit/compiler/regress-4389-3.js create mode 100644 test/mjsunit/compiler/regress-4389-4.js create mode 100644 test/mjsunit/compiler/regress-4389-5.js create mode 100644 test/mjsunit/compiler/regress-4389-6.js diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 077e9167e..ddcf21a4d 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); diff --git a/test/mjsunit/compiler/regress-4389-1.js b/test/mjsunit/compiler/regress-4389-1.js new file mode 100644 index 000000000..c58ce2da4 --- /dev/null +++ b/test/mjsunit/compiler/regress-4389-1.js @@ -0,0 +1,11 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --dead-code-elimination + +function foo(x) { Math.fround(x); } +foo(1); +foo(2); +%OptimizeFunctionOnNextCall(foo); +assertThrows(function() { foo(Symbol()) }, TypeError); diff --git a/test/mjsunit/compiler/regress-4389-2.js b/test/mjsunit/compiler/regress-4389-2.js new file mode 100644 index 000000000..3b720a5a9 --- /dev/null +++ b/test/mjsunit/compiler/regress-4389-2.js @@ -0,0 +1,11 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --dead-code-elimination + +function foo(x) { Math.sqrt(x); } +foo(1); +foo(2); +%OptimizeFunctionOnNextCall(foo); +assertThrows(function() { foo(Symbol()) }, TypeError); diff --git a/test/mjsunit/compiler/regress-4389-3.js b/test/mjsunit/compiler/regress-4389-3.js new file mode 100644 index 000000000..9aa72d1ac --- /dev/null +++ b/test/mjsunit/compiler/regress-4389-3.js @@ -0,0 +1,11 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --dead-code-elimination + +function foo(x) { Math.floor(x); } +foo(1); +foo(2); +%OptimizeFunctionOnNextCall(foo); +assertThrows(function() { foo(Symbol()) }, TypeError); diff --git a/test/mjsunit/compiler/regress-4389-4.js b/test/mjsunit/compiler/regress-4389-4.js new file mode 100644 index 000000000..e824973fa --- /dev/null +++ b/test/mjsunit/compiler/regress-4389-4.js @@ -0,0 +1,11 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --dead-code-elimination + +function foo(x) { Math.round(x); } +foo(1); +foo(2); +%OptimizeFunctionOnNextCall(foo); +assertThrows(function() { foo(Symbol()) }, TypeError); diff --git a/test/mjsunit/compiler/regress-4389-5.js b/test/mjsunit/compiler/regress-4389-5.js new file mode 100644 index 000000000..64797bc76 --- /dev/null +++ b/test/mjsunit/compiler/regress-4389-5.js @@ -0,0 +1,11 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --dead-code-elimination + +function foo(x) { Math.abs(x); } +foo(1); +foo(2); +%OptimizeFunctionOnNextCall(foo); +assertThrows(function() { foo(Symbol()) }, TypeError); diff --git a/test/mjsunit/compiler/regress-4389-6.js b/test/mjsunit/compiler/regress-4389-6.js new file mode 100644 index 000000000..fe065707f --- /dev/null +++ b/test/mjsunit/compiler/regress-4389-6.js @@ -0,0 +1,11 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax --dead-code-elimination + +function foo(x) { Math.log(x); } +foo(1); +foo(2); +%OptimizeFunctionOnNextCall(foo); +assertThrows(function() { foo(Symbol()) }, TypeError); -- 2.34.1