Reviewers: Sven Panne, Toon Verwaest,
Message:
@sven, this CL is a workaround fix for
https://code.google.com/p/v8/issues/detail?id=2898
@Toon, I move "HasNonSmiUse()" before observed_output_representation_ in
HBinaryOperation::InferRepresentation. The reason is in
IgnoreObservedOutputRepresentation, representation and related truncated
will be
checked. We need to adjust Smi to Int32 before that.
I also fix "(MinInt/-1) ^ 1" which would better not to check overflow
because of
its truncated int32 use.
Below is the performance data on my X86 PC.
opt orig
ss 218.2 218.3
kraken 1788 1785
Octane 15974 15804
Description:
Special handle for mul/div minus one when kAllUsesTruncatingToInt32
BUG=
Please review this at https://codereview.chromium.org/24521002/
SVN Base: git://github.com/v8/v8.git@master
Affected files (+54, -26 lines):
M src/hydrogen-instructions.h
M src/hydrogen-instructions.cc
A + test/mjsunit/div-mul-minus-one.js
Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index
a685198ba6288e54e0913695f6e6f4e5cc797f09..ca5c02497fd04e4969027d364d4c1ca8951a92d7
100644
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -1237,6 +1237,19 @@ HValue* HMul::Canonicalize() {
}
+bool HMul::MulMinusOne() {
+ ASSERT(left().IsSmiOrInteger32());
+ ASSERT(right().IsSmiOrInteger32());
+
+ if (left()->EqualsInteger32Constant(-1) ||
+ right()->EqualsInteger32Constant(-1)) {
+ return true;
+ }
+
+ return false;
+}
+
+
HValue* HMod::Canonicalize() {
return this;
}
@@ -1622,10 +1635,13 @@ Range* HMul::InferRange(Zone* zone) {
Range* a = left()->range();
Range* b = right()->range();
Range* res = a->Copy(zone);
- if (!res->MulAndCheckOverflow(r, b)) {
- // Clearing the kCanOverflow flag when kAllUsesAreTruncatingToInt32
- // would be wrong, because truncated integer multiplication is too
- // precise and therefore not the same as converting to Double and
back.
+ if (!res->MulAndCheckOverflow(r, b) ||
+ (((r.IsInteger32() && CheckFlag(kAllUsesTruncatingToInt32)) ||
+ (r.IsSmi() && CheckFlag(kAllUsesTruncatingToSmi))) &&
+ MulMinusOne())) {
+ // Truncated int multiplication is too precise and therefore not the
+ // same as converting to Double and back.
+ // Special handle for truncated int multiplication with minus one.
ClearFlag(kCanOverflow);
}
res->set_can_be_minus_zero(!CheckFlag(kAllUsesTruncatingToSmi) &&
@@ -1647,7 +1663,10 @@ Range* HDiv::InferRange(Zone* zone) {
result->set_can_be_minus_zero(!CheckFlag(kAllUsesTruncatingToInt32) &&
(a->CanBeMinusZero() ||
(a->CanBeZero() &&
b->CanBeNegative())));
- if (!a->Includes(kMinInt) || !b->Includes(-1)) {
+ if (!a->Includes(kMinInt) ||
+ !b->Includes(-1) ||
+ CheckFlag(kAllUsesTruncatingToInt32)) {
+ // It is safe to clear kCanOverflow when kAllUsesTruncatingToInt32.
ClearFlag(HValue::kCanOverflow);
}
@@ -2620,6 +2639,12 @@ void
HBinaryOperation::InferRepresentation(HInferRepresentationPhase* h_infer) {
ASSERT(CheckFlag(kFlexibleRepresentation));
Representation new_rep = RepresentationFromInputs();
UpdateRepresentation(new_rep, h_infer, "inputs");
+
+ if (representation().IsSmi() && HasNonSmiUse()) {
+ UpdateRepresentation(
+ Representation::Integer32(), h_infer, "use requirements");
+ }
+
if (observed_output_representation_.IsNone()) {
new_rep = RepresentationFromUses();
UpdateRepresentation(new_rep, h_infer, "uses");
@@ -2627,11 +2652,6 @@ void
HBinaryOperation::InferRepresentation(HInferRepresentationPhase* h_infer) {
new_rep = RepresentationFromOutput();
UpdateRepresentation(new_rep, h_infer, "output");
}
-
- if (representation().IsSmi() && HasNonSmiUse()) {
- UpdateRepresentation(
- Representation::Integer32(), h_infer, "use requirements");
- }
}
@@ -2657,8 +2677,9 @@ bool
HBinaryOperation::IgnoreObservedOutputRepresentation(
Representation current_rep) {
return ((current_rep.IsInteger32() &&
CheckUsesForFlag(kTruncatingToInt32)) ||
(current_rep.IsSmi() && CheckUsesForFlag(kTruncatingToSmi))) &&
- // Mul in Integer32 mode would be too precise.
- !this->IsMul();
+ // Mul in Integer32 mode would be too precise.
+ (!this->IsMul() ||
+ HMul::cast(this)->MulMinusOne());
}
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
8cb2f591742aebec491d57d4ab9305b51cc1eeb4..3f88a61c7783fb0ee5d4d433e93e5dfb8d6d50cb
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -4625,6 +4625,8 @@ class HMul V8_FINAL : public
HArithmeticBinaryOperation {
HArithmeticBinaryOperation::UpdateRepresentation(new_rep, h_infer,
reason);
}
+ bool MulMinusOne();
+
DECLARE_CONCRETE_INSTRUCTION(Mul)
protected:
Index: test/mjsunit/div-mul-minus-one.js
diff --git a/test/mjsunit/regress/regress-2132.js
b/test/mjsunit/div-mul-minus-one.js
similarity index 79%
copy from test/mjsunit/regress/regress-2132.js
copy to test/mjsunit/div-mul-minus-one.js
index
9eb2dc5b073e6e3c2ce46362903d07d727de3d27..f05bf0f54c68faf8f5e5cb5f35d7a8dcbc3fb02f
100644
--- a/test/mjsunit/regress/regress-2132.js
+++ b/test/mjsunit/div-mul-minus-one.js
@@ -27,22 +27,27 @@
// Flags: --allow-natives-syntax
-function mul(x, y) {
- return (x * y) | 0;
+function div(g) {
+ return (g/-1) ^ 1
}
-mul(0, 0);
-mul(0, 0);
-%OptimizeFunctionOnNextCall(mul);
-assertEquals(0, mul(0, -1));
-assertOptimized(mul);
+var kMinInt = 1 << 31;
+var expected_MinInt = div(kMinInt);
+var expected_minus_zero = div(0);
+%OptimizeFunctionOnNextCall(div);
+assertEquals(expected_MinInt, div(kMinInt));
+assertOptimized(div);
+assertEquals(expected_minus_zero , div(0));
+assertOptimized(div);
-function div(x, y) {
- return (x / y) | 0;
+function mul(g) {
+ return (g * -1) ^ 1
}
-div(4, 2);
-div(4, 2);
-%OptimizeFunctionOnNextCall(div);
-assertEquals(1, div(5, 3));
-assertOptimized(div);
+expected_MinInt = mul(kMinInt);
+expected_minus_zero = mul(0);
+%OptimizeFunctionOnNextCall(mul);
+assertEquals(expected_MinInt, mul(kMinInt));
+assertOptimized(mul);
+assertEquals(expected_minus_zero , mul(0));
+assertOptimized(mul);
--
--
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.