Revision: 15060
Author: [email protected]
Date: Tue Jun 11 04:43:57 2013
Log: Skip some conditional deopts for Div/Mul when all uses are
truncating.
- set "can be minus zero" flag properly so minus-zero checks are skipped
- skip "integer result?" check in division code when uses are truncating
- drive-by cleanup: consolidated computation of kCanOverflow flag for
Add/Sub into range inference phase
BUG=v8:2132
[email protected]
Review URL: https://codereview.chromium.org/16741002
http://code.google.com/p/v8/source/detail?r=15060
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-2132.js
Modified:
/branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/hydrogen-instructions.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-2132.js Tue Jun 11
04:43:57 2013
@@ -0,0 +1,48 @@
+// Copyright 2013 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 mul(x, y) {
+ return (x * y) | 0;
+}
+
+mul(0, 0);
+mul(0, 0);
+%OptimizeFunctionOnNextCall(mul);
+assertEquals(0, mul(0, -1));
+assertTrue(%GetOptimizationStatus(mul) != 2);
+
+function div(x, y) {
+ return (x / y) | 0;
+}
+
+div(4, 2);
+div(4, 2);
+%OptimizeFunctionOnNextCall(div);
+assertEquals(1, div(5, 3));
+assertTrue(%GetOptimizationStatus(div) != 2);
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Jun 11
03:47:44 2013
+++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Jun 11
04:43:57 2013
@@ -1419,8 +1419,7 @@
void LCodeGen::DoDivI(LDivI* instr) {
if (instr->hydrogen()->HasPowerOf2Divisor()) {
Register dividend = ToRegister(instr->left());
- int32_t divisor =
- HConstant::cast(instr->hydrogen()->right())->Integer32Value();
+ int32_t divisor = instr->hydrogen()->right()->GetInteger32Constant();
int32_t test_value = 0;
int32_t power = 0;
@@ -1443,10 +1442,19 @@
}
if (test_value != 0) {
- // Deoptimize if remainder is not 0.
- __ tst(dividend, Operand(test_value));
- DeoptimizeIf(ne, instr->environment());
- __ mov(dividend, Operand(dividend, ASR, power));
+ if (instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ __ cmp(dividend, Operand(0));
+ __ rsb(dividend, dividend, Operand(0), LeaveCC, lt);
+ __ mov(dividend, Operand(dividend, ASR, power));
+ if (divisor > 0) __ rsb(dividend, dividend, Operand(0), LeaveCC,
lt);
+ return; // Don't fall through to "__ rsb" below.
+ } else {
+ // Deoptimize if remainder is not 0.
+ __ tst(dividend, Operand(test_value));
+ DeoptimizeIf(ne, instr->environment());
+ __ mov(dividend, Operand(dividend, ASR, power));
+ }
}
if (divisor < 0) __ rsb(dividend, dividend, Operand(0));
@@ -1487,11 +1495,14 @@
CpuFeatureScope scope(masm(), SUDIV);
__ sdiv(result, left, right);
- // Compute remainder and deopt if it's not zero.
- const Register remainder = scratch0();
- __ mls(remainder, result, right, left);
- __ cmp(remainder, Operand::Zero());
- DeoptimizeIf(ne, instr->environment());
+ if (!instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ // Compute remainder and deopt if it's not zero.
+ const Register remainder = scratch0();
+ __ mls(remainder, result, right, left);
+ __ cmp(remainder, Operand::Zero());
+ DeoptimizeIf(ne, instr->environment());
+ }
} else {
const DoubleRegister vleft = ToDoubleRegister(instr->temp());
const DoubleRegister vright = double_scratch0();
@@ -1500,14 +1511,17 @@
__ vcvt_f64_s32(vleft, vleft.low());
__ vcvt_f64_s32(vright, vright.low());
__ vdiv(vleft, vleft, vright); // vleft now contains the result.
-
- // Convert back to integer32; deopt if exact conversion is not
possible.
- // Use vright as scratch register.
__ vcvt_s32_f64(vright.low(), vleft);
__ vmov(result, vright.low());
- __ vcvt_f64_s32(vright, vright.low());
- __ VFPCompareAndSetFlags(vleft, vright);
- DeoptimizeIf(ne, instr->environment());
+
+ if (!instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ // Deopt if exact conversion to integer was not possible.
+ // Use vright as scratch register.
+ __ vcvt_f64_s32(vright, vright.low());
+ __ VFPCompareAndSetFlags(vleft, vright);
+ DeoptimizeIf(ne, instr->environment());
+ }
}
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Tue Jun 11
03:47:44 2013
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Tue Jun 11
04:43:57 2013
@@ -526,6 +526,17 @@
}
return true;
}
+
+
+bool HValue::HasAtLeastOneUseWithFlagAndNoneWithout(Flag f) {
+ bool return_value = false;
+ for (HUseIterator it(uses()); !it.Done(); it.Advance()) {
+ if (it.value()->IsSimulate()) continue;
+ if (!it.value()->CheckFlag(f)) return false;
+ return_value = true;
+ }
+ return return_value;
+}
HUseIterator::HUseIterator(HUseListNode* head) : next_(head) {
@@ -1427,14 +1438,6 @@
}
return this;
}
-
-
-HValue* HArithmeticBinaryOperation::Canonicalize() {
- if (representation().IsInteger32() &&
CheckUsesForFlag(kTruncatingToInt32)) {
- ClearFlag(kCanOverflow);
- }
- return this;
-}
static bool IsIdentityOperation(HValue* arg1, HValue* arg2, int32_t
identity) {
@@ -1446,13 +1449,13 @@
HValue* HAdd::Canonicalize() {
if (IsIdentityOperation(left(), right(), 0)) return left();
if (IsIdentityOperation(right(), left(), 0)) return right();
- return HArithmeticBinaryOperation::Canonicalize();
+ return this;
}
HValue* HSub::Canonicalize() {
if (IsIdentityOperation(left(), right(), 0)) return left();
- return HArithmeticBinaryOperation::Canonicalize();
+ return this;
}
@@ -1758,11 +1761,13 @@
Range* a = left()->range();
Range* b = right()->range();
Range* res = a->Copy(zone);
- if (!res->AddAndCheckOverflow(b)) {
+ if (!res->AddAndCheckOverflow(b) ||
+ CheckFlag(kAllUsesTruncatingToInt32)) {
ClearFlag(kCanOverflow);
}
- bool m0 = a->CanBeMinusZero() && b->CanBeMinusZero();
- res->set_can_be_minus_zero(m0);
+ if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+ res->set_can_be_minus_zero(a->CanBeMinusZero() &&
b->CanBeMinusZero());
+ }
return res;
} else {
return HValue::InferRange(zone);
@@ -1775,10 +1780,13 @@
Range* a = left()->range();
Range* b = right()->range();
Range* res = a->Copy(zone);
- if (!res->SubAndCheckOverflow(b)) {
+ if (!res->SubAndCheckOverflow(b) ||
+ CheckFlag(kAllUsesTruncatingToInt32)) {
ClearFlag(kCanOverflow);
}
- res->set_can_be_minus_zero(a->CanBeMinusZero() && b->CanBeZero());
+ if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+ res->set_can_be_minus_zero(a->CanBeMinusZero() && b->CanBeZero());
+ }
return res;
} else {
return HValue::InferRange(zone);
@@ -1792,11 +1800,16 @@
Range* b = right()->range();
Range* res = a->Copy(zone);
if (!res->MulAndCheckOverflow(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.
ClearFlag(kCanOverflow);
}
- bool m0 = (a->CanBeZero() && b->CanBeNegative()) ||
- (a->CanBeNegative() && b->CanBeZero());
- res->set_can_be_minus_zero(m0);
+ if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+ bool m0 = (a->CanBeZero() && b->CanBeNegative()) ||
+ (a->CanBeNegative() && b->CanBeZero());
+ res->set_can_be_minus_zero(m0);
+ }
return res;
} else {
return HValue::InferRange(zone);
@@ -1809,12 +1822,14 @@
Range* a = left()->range();
Range* b = right()->range();
Range* result = new(zone) Range();
- if (a->CanBeMinusZero()) {
- result->set_can_be_minus_zero(true);
- }
+ if (!CheckFlag(kAllUsesTruncatingToInt32)) {
+ if (a->CanBeMinusZero()) {
+ result->set_can_be_minus_zero(true);
+ }
- if (a->CanBeZero() && b->CanBeNegative()) {
- result->set_can_be_minus_zero(true);
+ if (a->CanBeZero() && b->CanBeNegative()) {
+ result->set_can_be_minus_zero(true);
+ }
}
if (!a->Includes(kMinInt) || !b->Includes(-1)) {
@@ -1846,7 +1861,7 @@
Range* result = new(zone) Range(left_can_be_negative ?
-positive_bound : 0,
a->CanBePositive() ? positive_bound :
0);
- if (left_can_be_negative) {
+ if (left_can_be_negative && !CheckFlag(kAllUsesTruncatingToInt32)) {
result->set_can_be_minus_zero(true);
}
@@ -2298,10 +2313,6 @@
current_rep.IsInteger32() &&
// Mul in Integer32 mode would be too precise.
!this->IsMul() &&
- // TODO(jkummerow): Remove blacklisting of Div when the Div
- // instruction has learned not to deopt when the remainder is
- // non-zero but all uses are truncating.
- !this->IsDiv() &&
CheckUsesForFlag(kTruncatingToInt32);
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Mon Jun 10 05:05:54
2013
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Tue Jun 11 04:43:57
2013
@@ -54,6 +54,7 @@
#define HYDROGEN_ABSTRACT_INSTRUCTION_LIST(V) \
+ V(ArithmeticBinaryOperation) \
V(BinaryOperation) \
V(BitwiseBinaryOperation) \
V(ControlInstruction) \
@@ -797,6 +798,7 @@
kAllowUndefinedAsNaN,
kIsArguments,
kTruncatingToInt32,
+ kAllUsesTruncatingToInt32,
// Set after an instruction is killed.
kIsDead,
// Instructions that are allowed to produce full range unsigned integer
@@ -996,6 +998,9 @@
// Returns true if the flag specified is set for all uses, false
otherwise.
bool CheckUsesForFlag(Flag f);
+ // Returns true if the flag specified is set for all uses, and this set
+ // of uses is non-empty.
+ bool HasAtLeastOneUseWithFlagAndNoneWithout(Flag f);
GVNFlagSet gvn_flags() const { return gvn_flags_; }
void SetGVNFlag(GVNFlag f) { gvn_flags_.Add(f); }
@@ -3856,7 +3861,7 @@
: representation();
}
- virtual HValue* Canonicalize();
+ DECLARE_ABSTRACT_INSTRUCTION(ArithmeticBinaryOperation)
private:
virtual bool IsDeletable() const { return true; }
@@ -4488,9 +4493,8 @@
HValue* right);
bool HasPowerOf2Divisor() {
- if (right()->IsConstant() &&
- HConstant::cast(right())->HasInteger32Value()) {
- int32_t value = HConstant::cast(right())->Integer32Value();
+ if (right()->IsInteger32Constant()) {
+ int32_t value = right()->GetInteger32Constant();
return value != 0 && (IsPowerOf2(value) || IsPowerOf2(-value));
}
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Mon Jun 10 08:47:23 2013
+++ /branches/bleeding_edge/src/hydrogen.cc Tue Jun 11 04:43:57 2013
@@ -2091,6 +2091,22 @@
void HGraph::Canonicalize() {
HPhase phase("H_Canonicalize", this);
+ // Before removing no-op instructions, save their semantic value.
+ // We must be careful not to set the flag unnecessarily, because GVN
+ // cannot identify two instructions when their flag value differs.
+ for (int i = 0; i < blocks()->length(); ++i) {
+ HInstruction* instr = blocks()->at(i)->first();
+ while (instr != NULL) {
+ if (instr->IsArithmeticBinaryOperation() &&
+ instr->representation().IsInteger32() &&
+ instr->HasAtLeastOneUseWithFlagAndNoneWithout(
+ HInstruction::kTruncatingToInt32)) {
+ instr->SetFlag(HInstruction::kAllUsesTruncatingToInt32);
+ }
+ instr = instr->next();
+ }
+ }
+ // Perform actual Canonicalization pass.
for (int i = 0; i < blocks()->length(); ++i) {
HInstruction* instr = blocks()->at(i)->first();
while (instr != NULL) {
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Mon Jun 10
05:05:54 2013
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Tue Jun 11
04:43:57 2013
@@ -1338,8 +1338,7 @@
void LCodeGen::DoDivI(LDivI* instr) {
if (!instr->is_flooring() && instr->hydrogen()->HasPowerOf2Divisor()) {
Register dividend = ToRegister(instr->left());
- int32_t divisor =
- HConstant::cast(instr->hydrogen()->right())->Integer32Value();
+ int32_t divisor = instr->hydrogen()->right()->GetInteger32Constant();
int32_t test_value = 0;
int32_t power = 0;
@@ -1362,10 +1361,26 @@
}
if (test_value != 0) {
- // Deoptimize if remainder is not 0.
- __ test(dividend, Immediate(test_value));
- DeoptimizeIf(not_zero, instr->environment());
- __ sar(dividend, power);
+ if (instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ Label done, negative;
+ __ cmp(dividend, 0);
+ __ j(less, &negative, Label::kNear);
+ __ sar(dividend, power);
+ __ jmp(&done, Label::kNear);
+
+ __ bind(&negative);
+ __ neg(dividend);
+ __ sar(dividend, power);
+ if (divisor > 0) __ neg(dividend);
+ __ bind(&done);
+ return; // Don't fall through to "__ neg" below.
+ } else {
+ // Deoptimize if remainder is not 0.
+ __ test(dividend, Immediate(test_value));
+ DeoptimizeIf(not_zero, instr->environment());
+ __ sar(dividend, power);
+ }
}
if (divisor < 0) __ neg(dividend);
@@ -1412,11 +1427,7 @@
__ cdq();
__ idiv(right_reg);
- if (!instr->is_flooring()) {
- // Deoptimize if remainder is not 0.
- __ test(edx, Operand(edx));
- DeoptimizeIf(not_zero, instr->environment());
- } else {
+ if (instr->is_flooring()) {
Label done;
__ test(edx, edx);
__ j(zero, &done, Label::kNear);
@@ -1424,6 +1435,11 @@
__ sar(edx, 31);
__ add(eax, edx);
__ bind(&done);
+ } else if (!instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ // Deoptimize if remainder is not 0.
+ __ test(edx, Operand(edx));
+ DeoptimizeIf(not_zero, instr->environment());
}
}
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Mon Jun 10
05:05:54 2013
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Tue Jun 11
04:43:57 2013
@@ -1257,10 +1257,26 @@
}
if (test_value != 0) {
- // Deoptimize if remainder is not 0.
- __ testl(dividend, Immediate(test_value));
- DeoptimizeIf(not_zero, instr->environment());
- __ sarl(dividend, Immediate(power));
+ if (instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ Label done, negative;
+ __ cmpl(dividend, Immediate(0));
+ __ j(less, &negative, Label::kNear);
+ __ sarl(dividend, Immediate(power));
+ __ jmp(&done, Label::kNear);
+
+ __ bind(&negative);
+ __ negl(dividend);
+ __ sarl(dividend, Immediate(power));
+ if (divisor > 0) __ negl(dividend);
+ __ bind(&done);
+ return; // Don't fall through to "__ neg" below.
+ } else {
+ // Deoptimize if remainder is not 0.
+ __ testl(dividend, Immediate(test_value));
+ DeoptimizeIf(not_zero, instr->environment());
+ __ sarl(dividend, Immediate(power));
+ }
}
if (divisor < 0) __ negl(dividend);
@@ -1307,11 +1323,7 @@
__ cdq();
__ idivl(right_reg);
- if (!instr->is_flooring()) {
- // Deoptimize if remainder is not 0.
- __ testl(rdx, rdx);
- DeoptimizeIf(not_zero, instr->environment());
- } else {
+ if (instr->is_flooring()) {
Label done;
__ testl(rdx, rdx);
__ j(zero, &done, Label::kNear);
@@ -1319,6 +1331,11 @@
__ sarl(rdx, Immediate(31));
__ addl(rax, rdx);
__ bind(&done);
+ } else if (!instr->hydrogen()->CheckFlag(
+ HInstruction::kAllUsesTruncatingToInt32)) {
+ // Deoptimize if remainder is not 0.
+ __ testl(rdx, rdx);
+ DeoptimizeIf(not_zero, instr->environment());
}
}
--
--
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.
