From 23fc5b75a8ca64e77e5321a29feb38a0ea1026cd Mon Sep 17 00:00:00 2001 From: "svenpanne@chromium.org" Date: Wed, 11 Jun 2014 13:29:25 +0000 Subject: [PATCH] Fixed flooring division by a power of 2, once again... Avoid right shifts by zero bits: On ARM it actually means shifting by 32 bits (correctness issue) and on other platforms they are useless (performance issue). This is fix for the fix in r20544. BUG=v8:3259 LOG=y R=yangguo@chromium.org Review URL: https://codereview.chromium.org/324403003 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21769 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-codegen-arm.cc | 16 +++++++++------- src/arm64/lithium-codegen-arm64.cc | 16 +++++++++------- src/ia32/lithium-codegen-ia32.cc | 13 ++++++++----- src/x64/lithium-codegen-x64.cc | 15 ++++++++------- test/mjsunit/mjsunit.status | 4 ---- 5 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 800f67b..249c22f 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -1469,20 +1469,22 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { DeoptimizeIf(eq, instr->environment()); } - // If the negation could not overflow, simply shifting is OK. - if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { - __ mov(result, Operand(dividend, ASR, shift)); + // Dividing by -1 is basically negation, unless we overflow. + if (divisor == -1) { + if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + DeoptimizeIf(vs, instr->environment()); + } return; } - // Dividing by -1 is basically negation, unless we overflow. - if (divisor == -1) { - DeoptimizeIf(vs, instr->environment()); + // If the negation could not overflow, simply shifting is OK. + if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + __ mov(result, Operand(result, ASR, shift)); return; } __ mov(result, Operand(kMinInt / divisor), LeaveCC, vs); - __ mov(result, Operand(dividend, ASR, shift), LeaveCC, vc); + __ mov(result, Operand(result, ASR, shift), LeaveCC, vc); } diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index f301c8b..c86efbd 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -3933,19 +3933,21 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { DeoptimizeIf(eq, instr->environment()); } - // If the negation could not overflow, simply shifting is OK. - if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { - __ Mov(result, Operand(dividend, ASR, shift)); + // Dividing by -1 is basically negation, unless we overflow. + if (divisor == -1) { + if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + DeoptimizeIf(vs, instr->environment()); + } return; } - // Dividing by -1 is basically negation, unless we overflow. - if (divisor == -1) { - DeoptimizeIf(vs, instr->environment()); + // If the negation could not overflow, simply shifting is OK. + if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + __ Mov(result, Operand(dividend, ASR, shift)); return; } - __ Asr(result, dividend, shift); + __ Asr(result, result, shift); __ Csel(result, result, kMinInt / divisor, vc); } diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 791faa2..1e4f756 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -1353,14 +1353,17 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { DeoptimizeIf(zero, instr->environment()); } - if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { - __ sar(dividend, shift); + // Dividing by -1 is basically negation, unless we overflow. + if (divisor == -1) { + if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + DeoptimizeIf(overflow, instr->environment()); + } return; } - // Dividing by -1 is basically negation, unless we overflow. - if (divisor == -1) { - DeoptimizeIf(overflow, instr->environment()); + // If the negation could not overflow, simply shifting is OK. + if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + __ sar(dividend, shift); return; } diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 4132134..809df5d 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -1134,16 +1134,17 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { DeoptimizeIf(zero, instr->environment()); } - // If the negation could not overflow, simply shifting is OK. - if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { - __ sarl(dividend, Immediate(shift)); + // Dividing by -1 is basically negation, unless we overflow. + if (divisor == -1) { + if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + DeoptimizeIf(overflow, instr->environment()); + } return; } - // Note that we could emit branch-free code, but that would need one more - // register. - if (divisor == -1) { - DeoptimizeIf(overflow, instr->environment()); + // If the negation could not overflow, simply shifting is OK. + if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) { + __ sarl(dividend, Immediate(shift)); return; } diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 70653f6..3a6b281 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -308,10 +308,6 @@ # Currently always deopt on minus zero 'math-floor-of-div-minus-zero': [SKIP], - # Issue 3259. - 'math-floor-of-div-nosudiv': [PASS, FAIL], - 'math-floor-of-div': [PASS, FAIL], - ############################################################################ # Slow tests. 'regress/regress-2185-2': [PASS, SLOW], -- 2.7.4