From 82f0649c762865b16afe3413e275abe59014fdac Mon Sep 17 00:00:00 2001 From: "rodolph.perfetta@gmail.com" Date: Fri, 6 Sep 2013 13:12:46 +0000 Subject: [PATCH] ARM: Improve integer multiplication. TEST=test/mjsunit/lithium/MulI.js R=bmeurer@chromium.org Review URL: https://codereview.chromium.org/23452022 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16576 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 43 ++++++++++++++++++-------- src/arm/lithium-arm.h | 6 ++-- src/arm/lithium-codegen-arm.cc | 57 +++++++++++++++++----------------- test/mjsunit/lithium/MulI.js | 70 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 test/mjsunit/lithium/MulI.js diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index f9e21f7..132e1a6 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -1511,20 +1511,39 @@ LInstruction* LChunkBuilder::DoMul(HMul* instr) { if (instr->representation().IsSmiOrInteger32()) { ASSERT(instr->left()->representation().Equals(instr->representation())); ASSERT(instr->right()->representation().Equals(instr->representation())); - LOperand* left; - LOperand* right = UseOrConstant(instr->BetterRightOperand()); - LOperand* temp = NULL; - if (instr->CheckFlag(HValue::kBailoutOnMinusZero) && - (instr->CheckFlag(HValue::kCanOverflow) || - !right->IsConstantOperand())) { - left = UseRegister(instr->BetterLeftOperand()); - temp = TempRegister(); + HValue* left = instr->BetterLeftOperand(); + HValue* right = instr->BetterRightOperand(); + LOperand* left_op; + LOperand* right_op; + bool can_overflow = instr->CheckFlag(HValue::kCanOverflow); + bool bailout_on_minus_zero = instr->CheckFlag(HValue::kBailoutOnMinusZero); + + if (right->IsConstant()) { + HConstant* constant = HConstant::cast(right); + int32_t constant_value = constant->Integer32Value(); + // Constants -1, 0 and 1 can be optimized if the result can overflow. + // For other constants, it can be optimized only without overflow. + if (!can_overflow || ((constant_value >= -1) && (constant_value <= 1))) { + left_op = UseRegisterAtStart(left); + right_op = UseConstant(right); + } else { + if (bailout_on_minus_zero) { + left_op = UseRegister(left); + } else { + left_op = UseRegisterAtStart(left); + } + right_op = UseRegister(right); + } } else { - left = UseRegisterAtStart(instr->BetterLeftOperand()); + if (bailout_on_minus_zero) { + left_op = UseRegister(left); + } else { + left_op = UseRegisterAtStart(left); + } + right_op = UseRegister(right); } - LMulI* mul = new(zone()) LMulI(left, right, temp); - if (instr->CheckFlag(HValue::kCanOverflow) || - instr->CheckFlag(HValue::kBailoutOnMinusZero)) { + LMulI* mul = new(zone()) LMulI(left_op, right_op); + if (can_overflow || bailout_on_minus_zero) { AssignEnvironment(mul); } return DefineAsRegister(mul); diff --git a/src/arm/lithium-arm.h b/src/arm/lithium-arm.h index 781dcf4..b68dbe7 100644 --- a/src/arm/lithium-arm.h +++ b/src/arm/lithium-arm.h @@ -677,17 +677,15 @@ class LMathFloorOfDiv V8_FINAL : public LTemplateInstruction<1, 2, 1> { }; -class LMulI V8_FINAL : public LTemplateInstruction<1, 2, 1> { +class LMulI V8_FINAL : public LTemplateInstruction<1, 2, 0> { public: - LMulI(LOperand* left, LOperand* right, LOperand* temp) { + LMulI(LOperand* left, LOperand* right) { inputs_[0] = left; inputs_[1] = right; - temps_[0] = temp; } LOperand* left() { return inputs_[0]; } LOperand* right() { return inputs_[1]; } - LOperand* temp() { return temps_[0]; } DECLARE_CONCRETE_INSTRUCTION(MulI, "mul-i") DECLARE_HYDROGEN_ACCESSOR(Mul) diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index ae24210..bc34cfa 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -1573,17 +1573,16 @@ void LCodeGen::DoMathFloorOfDiv(LMathFloorOfDiv* instr) { void LCodeGen::DoMulI(LMulI* instr) { - Register scratch = scratch0(); Register result = ToRegister(instr->result()); // Note that result may alias left. Register left = ToRegister(instr->left()); LOperand* right_op = instr->right(); - bool can_overflow = instr->hydrogen()->CheckFlag(HValue::kCanOverflow); bool bailout_on_minus_zero = instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero); + bool overflow = instr->hydrogen()->CheckFlag(HValue::kCanOverflow); - if (right_op->IsConstantOperand() && !can_overflow) { + if (right_op->IsConstantOperand()) { int32_t constant = ToInteger32(LConstantOperand::cast(right_op)); if (bailout_on_minus_zero && (constant < 0)) { @@ -1595,7 +1594,12 @@ void LCodeGen::DoMulI(LMulI* instr) { switch (constant) { case -1: - __ rsb(result, left, Operand::Zero()); + if (overflow) { + __ rsb(result, left, Operand::Zero(), SetCC); + DeoptimizeIf(vs, instr->environment()); + } else { + __ rsb(result, left, Operand::Zero()); + } break; case 0: if (bailout_on_minus_zero) { @@ -1616,23 +1620,21 @@ void LCodeGen::DoMulI(LMulI* instr) { int32_t mask = constant >> 31; uint32_t constant_abs = (constant + mask) ^ mask; - if (IsPowerOf2(constant_abs) || - IsPowerOf2(constant_abs - 1) || - IsPowerOf2(constant_abs + 1)) { - if (IsPowerOf2(constant_abs)) { - int32_t shift = WhichPowerOf2(constant_abs); - __ mov(result, Operand(left, LSL, shift)); - } else if (IsPowerOf2(constant_abs - 1)) { - int32_t shift = WhichPowerOf2(constant_abs - 1); - __ add(result, left, Operand(left, LSL, shift)); - } else if (IsPowerOf2(constant_abs + 1)) { - int32_t shift = WhichPowerOf2(constant_abs + 1); - __ rsb(result, left, Operand(left, LSL, shift)); - } - + if (IsPowerOf2(constant_abs)) { + int32_t shift = WhichPowerOf2(constant_abs); + __ mov(result, Operand(left, LSL, shift)); + // Correct the sign of the result is the constant is negative. + if (constant < 0) __ rsb(result, result, Operand::Zero()); + } else if (IsPowerOf2(constant_abs - 1)) { + int32_t shift = WhichPowerOf2(constant_abs - 1); + __ add(result, left, Operand(left, LSL, shift)); + // Correct the sign of the result is the constant is negative. + if (constant < 0) __ rsb(result, result, Operand::Zero()); + } else if (IsPowerOf2(constant_abs + 1)) { + int32_t shift = WhichPowerOf2(constant_abs + 1); + __ rsb(result, left, Operand(left, LSL, shift)); // Correct the sign of the result is the constant is negative. if (constant < 0) __ rsb(result, result, Operand::Zero()); - } else { // Generate standard code. __ mov(ip, Operand(constant)); @@ -1641,12 +1643,11 @@ void LCodeGen::DoMulI(LMulI* instr) { } } else { - Register right = EmitLoadRegister(right_op, scratch); - if (bailout_on_minus_zero) { - __ orr(ToRegister(instr->temp()), left, right); - } + ASSERT(right_op->IsRegister()); + Register right = ToRegister(right_op); - if (can_overflow) { + if (overflow) { + Register scratch = scratch0(); // scratch:result = left * right. if (instr->hydrogen()->representation().IsSmi()) { __ SmiUntag(result, left); @@ -1666,12 +1667,12 @@ void LCodeGen::DoMulI(LMulI* instr) { } if (bailout_on_minus_zero) { - // Bail out if the result is supposed to be negative zero. Label done; + __ teq(left, Operand(right)); + __ b(pl, &done); + // Bail out if the result is minus zero. __ cmp(result, Operand::Zero()); - __ b(ne, &done); - __ cmp(ToRegister(instr->temp()), Operand::Zero()); - DeoptimizeIf(mi, instr->environment()); + DeoptimizeIf(eq, instr->environment()); __ bind(&done); } } diff --git a/test/mjsunit/lithium/MulI.js b/test/mjsunit/lithium/MulI.js new file mode 100644 index 0000000..68588bd --- /dev/null +++ b/test/mjsunit/lithium/MulI.js @@ -0,0 +1,70 @@ +// 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 --no-use-osr + +function foo_smi(a, b) { + var result = a * b; + result += a * 35; + result += a * -1; + result += a * 1; + result += a * 0; + return result * a; +} + +function foo_int(a, b) { + var result = a * b; + result += a * 35; + result += a * -1; + result += a * 1; + result += a * 0; + return result * a; +} + +foo_smi(10, 5); +var r1 = foo_smi(10, 5); +%OptimizeFunctionOnNextCall(foo_smi); +var r2 = foo_smi(10, 5); + +assertEquals(r1, r2); + +foo_int(10, 21474800); +var r3 = foo_int(10, 21474800); +%OptimizeFunctionOnNextCall(foo_int); +var r4 = foo_int(10, 21474800); + +assertEquals(r3, r4); + +// Check overflow with -1 constant. +function foo2(value) { + return value * -1; +} + +foo2(-2147483600); +foo2(-2147483600); +%OptimizeFunctionOnNextCall(foo2); +assertEquals(2147483648, foo2(-2147483648)); -- 2.7.4