Fixed floor-of-div optimization.
authorsvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 22 Jan 2014 11:54:51 +0000 (11:54 +0000)
committersvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 22 Jan 2014 11:54:51 +0000 (11:54 +0000)
We removed an HDiv by hand which was still used by an HChange. The
solution is letting dead code removal do the cleanup.

Removed a fragile "optimization" (looking through an HChange), too,
this obviously never triggered and is hard to get right given all our
global invariants and state/type/... changes.

The repro is a bit tricky, because you need inlining to make our
representations and types disagree in this case.

LOG=y
BUG=334708
R=bmeurer@chromium.org

Review URL: https://codereview.chromium.org/143903016

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18737 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/hydrogen-instructions.cc
test/mjsunit/regress/regress-334708.js [new file with mode: 0644]

index 062b5ae..453239b 100644 (file)
@@ -1442,7 +1442,6 @@ HValue* HUnaryMathOperation::Canonicalize() {
 
   if (op() == kMathFloor) {
     HValue* val = value();
-    if (val->IsChange()) val = HChange::cast(val)->value();
     if (val->IsDiv() && (val->UseCount() == 1)) {
       HDiv* hdiv = HDiv::cast(val);
       HValue* left = hdiv->left();
@@ -1481,17 +1480,8 @@ HValue* HUnaryMathOperation::Canonicalize() {
       }
       HMathFloorOfDiv* instr =
           HMathFloorOfDiv::New(block()->zone(), context(), new_left, new_right);
-      // Replace this HMathFloor instruction by the new HMathFloorOfDiv.
       instr->InsertBefore(this);
-      ReplaceAllUsesWith(instr);
-      Kill();
-      // We know the division had no other uses than this HMathFloor. Delete it.
-      // Dead code elimination will deal with |left| and |right| if
-      // appropriate.
-      hdiv->DeleteAndReplaceWith(NULL);
-
-      // Return NULL to remove this instruction from the graph.
-      return NULL;
+      return instr;
     }
   }
   return this;
diff --git a/test/mjsunit/regress/regress-334708.js b/test/mjsunit/regress/regress-334708.js
new file mode 100644 (file)
index 0000000..f0291bb
--- /dev/null
@@ -0,0 +1,42 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+function foo(x, y) {
+  return Math.floor(x / y);
+}
+
+function bar(x, y) {
+  return foo(x + 1, y + 1);
+}
+
+foo(16, "4");
+
+bar(64, 2);
+%OptimizeFunctionOnNextCall(bar);
+bar(64, 2);