Bug 832085 - Never bail in truncated LModI/LModPowTwoI/LModMaskI (r=hv1989)

--HG--
extra : rebase_source : efa4ec10c37fdec3861ac5097db7c5b5c2ade63b
This commit is contained in:
Luke Wagner 2012-12-07 18:54:05 -08:00
parent fd2cdad54e
commit f7323d0f91
9 changed files with 180 additions and 38 deletions

View File

@ -863,6 +863,31 @@ MMod::foldsTo(bool useValueNumbers)
return this;
}
void
MMod::analyzeTruncateBackward()
{
if (!isTruncated())
setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
}
bool
MMod::updateForReplacement(MDefinition *ins_)
{
JS_ASSERT(ins_->isMod());
MMod *ins = ins_->toMod();
if (isTruncated() && ins->isTruncated())
setTruncated(Max(isTruncated(), ins->isTruncated()));
else
setTruncated(0);
return true;
}
bool
MMod::fallible()
{
return !isTruncated();
}
void
MAdd::analyzeTruncateBackward()
{

View File

@ -2814,8 +2814,11 @@ class MDiv : public MBinaryArithInstruction
class MMod : public MBinaryArithInstruction
{
int implicitTruncate_;
MMod(MDefinition *left, MDefinition *right)
: MBinaryArithInstruction(left, right)
: MBinaryArithInstruction(left, right),
implicitTruncate_(0)
{
setResultType(MIRType_Value);
}
@ -2827,12 +2830,23 @@ class MMod : public MBinaryArithInstruction
}
MDefinition *foldsTo(bool useValueNumbers);
void analyzeTruncateBackward();
double getIdentity() {
JS_NOT_REACHED("not used");
return 1;
}
int isTruncated() const {
return implicitTruncate_;
}
void setTruncated(int truncate) {
implicitTruncate_ = truncate;
}
bool updateForReplacement(MDefinition *ins);
void computeRange();
bool fallible();
};
class MConcat

View File

@ -576,6 +576,8 @@ CodeGeneratorARM::visitModI(LModI *ins)
Register lhs = ToRegister(ins->lhs());
Register rhs = ToRegister(ins->rhs());
Register callTemp = ToRegister(ins->getTemp(2));
MMod *mir = ins->mir();
Label done;
// save the lhs in case we end up with a 0 that should be a -0.0 because lhs < 0.
JS_ASSERT(callTemp.code() > r3.code() && callTemp.code() < r12.code());
masm.ma_mov(lhs, callTemp);
@ -583,8 +585,18 @@ CodeGeneratorARM::visitModI(LModI *ins)
// The integer division will give INT_MIN, but we want -(double)INT_MIN.
masm.ma_cmp(lhs, Imm32(INT_MIN)); // sets EQ if lhs == INT_MIN
masm.ma_cmp(rhs, Imm32(-1), Assembler::Equal); // if EQ (LHS == INT_MIN), sets EQ if rhs == -1
if (!bailoutIf(Assembler::Equal, ins->snapshot()))
return false;
if (mir->isTruncated()) {
// (INT_MIN % -1)|0 == 0
Label skip;
masm.ma_b(&skip, Assembler::NotEqual);
masm.ma_mov(Imm32(0), r1);
masm.ma_b(&done);
masm.bind(&skip);
} else {
JS_ASSERT(mir->fallible());
if (!bailoutIf(Assembler::Equal, ins->snapshot()))
return false;
}
// 0/X (with X < 0) is bad because both of these values *should* be doubles, and
// the result should be -0.0, which cannot be represented in integers.
// X/0 is bad because it will give garbage (or abort), when it should give
@ -600,21 +612,35 @@ CodeGeneratorARM::visitModI(LModI *ins)
// if (Y > 0), we don't set EQ, and we don't trigger LT, so we don't take the bailout.
masm.ma_cmp(rhs, Imm32(0));
masm.ma_cmp(lhs, Imm32(0), Assembler::LessThan);
if (!bailoutIf(Assembler::Equal, ins->snapshot()))
return false;
if (mir->isTruncated()) {
// NaN|0 == 0 and (0 % -X)|0 == 0
Label skip;
masm.ma_b(&skip, Assembler::NotEqual);
masm.ma_mov(Imm32(0), r1);
masm.ma_b(&done);
masm.bind(&skip);
} else {
JS_ASSERT(mir->fallible());
if (!bailoutIf(Assembler::Equal, ins->snapshot()))
return false;
}
masm.setupAlignedABICall(2);
masm.passABIArg(lhs);
masm.passABIArg(rhs);
masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, __aeabi_idivmod));
// If X%Y == 0 and X < 0, then we *actually* wanted to return -0.0
Label join;
// See if X < 0
masm.ma_cmp(r1, Imm32(0));
masm.ma_b(&join, Assembler::NotEqual);
masm.ma_cmp(callTemp, Imm32(0));
if (!bailoutIf(Assembler::Signed, ins->snapshot()))
return false;
masm.bind(&join);
if (mir->isTruncated()) {
// -0.0|0 == 0
} else {
JS_ASSERT(mir->fallible());
masm.ma_b(&done, Assembler::NotEqual);
masm.ma_cmp(callTemp, Imm32(0));
if (!bailoutIf(Assembler::Signed, ins->snapshot()))
return false;
}
masm.bind(&done);
return true;
}
@ -623,6 +649,7 @@ CodeGeneratorARM::visitModPowTwoI(LModPowTwoI *ins)
{
Register in = ToRegister(ins->getOperand(0));
Register out = ToRegister(ins->getDef(0));
MMod *mir = ins->mir();
Label fin;
// bug 739870, jbramley has a different sequence that may help with speed here
masm.ma_mov(in, out, SetCond);
@ -630,8 +657,13 @@ CodeGeneratorARM::visitModPowTwoI(LModPowTwoI *ins)
masm.ma_rsb(Imm32(0), out, NoSetCond, Assembler::Signed);
masm.ma_and(Imm32((1<<ins->shift())-1), out);
masm.ma_rsb(Imm32(0), out, SetCond, Assembler::Signed);
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
if (!mir->isTruncated()) {
JS_ASSERT(mir->fallible());
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
} else {
// -0|0 == 0
}
masm.bind(&fin);
return true;
}
@ -642,9 +674,15 @@ CodeGeneratorARM::visitModMaskI(LModMaskI *ins)
Register src = ToRegister(ins->getOperand(0));
Register dest = ToRegister(ins->getDef(0));
Register tmp = ToRegister(ins->getTemp(0));
MMod *mir = ins->mir();
masm.ma_mod_mask(src, dest, tmp, ins->shift());
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
if (!mir->isTruncated()) {
JS_ASSERT(mir->fallible());
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
} else {
// -0|0 == 0
}
return true;
}
bool

View File

@ -123,6 +123,10 @@ class LModI : public LBinaryMath<3>
setTemp(1, temp2);
setTemp(2, callTemp);
}
MMod *mir() const {
return mir_->toMod();
}
};
class LModPowTwoI : public LInstructionHelper<1, 1, 0>
@ -141,6 +145,10 @@ class LModPowTwoI : public LInstructionHelper<1, 1, 0>
{
setOperand(0, lhs);
}
MMod *mir() const {
return mir_->toMod();
}
};
class LModMaskI : public LInstructionHelper<1, 1, 1>
@ -160,6 +168,10 @@ class LModMaskI : public LInstructionHelper<1, 1, 1>
int32_t shift() const {
return shift_;
}
MMod *mir() const {
return mir_->toMod();
}
};
class LPowHalfD : public LInstructionHelper<1, 1, 0>

View File

@ -260,18 +260,24 @@ LIRGeneratorARM::lowerModI(MMod *mod)
int32_t rhs = mod->rhs()->toConstant()->value().toInt32();
int32_t shift;
JS_FLOOR_LOG2(shift, rhs);
if (1 << shift == rhs) {
if (rhs > 0 && 1 << shift == rhs) {
LModPowTwoI *lir = new LModPowTwoI(useRegister(mod->lhs()), shift);
return (assignSnapshot(lir) && define(lir, mod));
if (mod->fallible() && !assignSnapshot(lir))
return false;
return define(lir, mod);
} else if (shift < 31 && (1 << (shift+1)) - 1 == rhs) {
LModMaskI *lir = new LModMaskI(useRegister(mod->lhs()), temp(LDefinition::GENERAL), shift+1);
return (assignSnapshot(lir) && define(lir, mod));
if (mod->fallible() && !assignSnapshot(lir))
return false;
return define(lir, mod);
}
}
LModI *lir = new LModI(useFixed(mod->lhs(), r0), use(mod->rhs(), r1),
tempFixed(r2), tempFixed(r3), temp(LDefinition::GENERAL));
return assignSnapshot(lir) && defineFixed(lir, mod, LAllocation(AnyRegister(r1)));
if (mod->fallible() && !assignSnapshot(lir))
return false;
return defineFixed(lir, mod, LAllocation(AnyRegister(r1)));
}
bool

View File

@ -129,7 +129,7 @@ CodeGeneratorX86Shared::visitTestDAndBranch(LTestDAndBranch *test)
// ucomisd flags:
// Z P C
// ---------
// ---------
// NaN 1 1 1
// > 0 0 0
// < 0 0 1
@ -266,7 +266,7 @@ CodeGeneratorX86Shared::generateOutOfLineCode()
if (deoptLabel_) {
// All non-table-based bailouts will go here.
masm.bind(deoptLabel_);
// Push the frame size, so the handler can recover the IonScript.
masm.push(Imm32(frameSize()));
@ -754,13 +754,14 @@ CodeGeneratorX86Shared::visitModPowTwoI(LModPowTwoI *ins)
{
Register lhs = ToRegister(ins->getOperand(0));
int32_t shift = ins->shift();
Label negative, join;
Label negative, done;
// Switch based on sign of the lhs.
// Positive numbers are just a bitmask
masm.branchTest32(Assembler::Signed, lhs, lhs, &negative);
{
masm.andl(Imm32((1 << shift) - 1), lhs);
masm.jump(&join);
masm.jump(&done);
}
// Negative numbers need a negate, bitmask, negate
{
@ -770,10 +771,10 @@ CodeGeneratorX86Shared::visitModPowTwoI(LModPowTwoI *ins)
masm.negl(lhs);
masm.andl(Imm32((1 << shift) - 1), lhs);
masm.negl(lhs);
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
if (!ins->mir()->isTruncated() && !bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
}
masm.bind(&join);
masm.bind(&done);
return true;
}
@ -795,12 +796,22 @@ CodeGeneratorX86Shared::visitModI(LModI *ins)
lhs = temp;
}
Label done;
// Prevent divide by zero
masm.testl(rhs, rhs);
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
if (ins->mir()->isTruncated()) {
Label notzero;
masm.j(Assembler::NonZero, &notzero);
masm.xorl(edx, edx);
masm.jmp(&done);
masm.bind(&notzero);
} else {
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
}
Label negative, done;
Label negative;
// Switch based on sign of the lhs.
masm.branchTest32(Assembler::Signed, lhs, lhs, &negative);
@ -821,17 +832,25 @@ CodeGeneratorX86Shared::visitModI(LModI *ins)
masm.cmpl(lhs, Imm32(INT32_MIN));
masm.j(Assembler::NotEqual, &notmin);
masm.cmpl(rhs, Imm32(-1));
if (!bailoutIf(Assembler::Equal, ins->snapshot()))
return false;
if (ins->mir()->isTruncated()) {
masm.j(Assembler::NotEqual, &notmin);
masm.xorl(edx, edx);
masm.jmp(&done);
} else {
if (!bailoutIf(Assembler::Equal, ins->snapshot()))
return false;
}
masm.bind(&notmin);
masm.cdq();
masm.idiv(rhs);
// A remainder of 0 means that the rval must be -0, which is a double.
masm.testl(remainder, remainder);
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
if (!ins->mir()->isTruncated()) {
// A remainder of 0 means that the rval must be -0, which is a double.
masm.testl(remainder, remainder);
if (!bailoutIf(Assembler::Zero, ins->snapshot()))
return false;
}
}
masm.bind(&done);
@ -1116,7 +1135,7 @@ CodeGeneratorX86Shared::visitFloor(LFloor *lir)
masm.cmp32(output, Imm32(INT_MIN));
if (!bailoutIf(Assembler::Equal, lir->snapshot()))
return false;
// Test whether the input double was integer-valued.
masm.cvtsi2sd(output, scratch);
masm.branchDouble(Assembler::DoubleEqualOrUnordered, input, scratch, &end);

View File

@ -44,6 +44,9 @@ class LModI : public LBinaryMath<1>
const LDefinition *remainder() {
return getDef(0);
}
MMod *mir() const {
return mir_->toMod();
}
};
class LModPowTwoI : public LInstructionHelper<1,1,0>
@ -65,6 +68,9 @@ class LModPowTwoI : public LInstructionHelper<1,1,0>
const LDefinition *remainder() {
return getDef(0);
}
MMod *mir() const {
return mir_->toMod();
}
};
// Double raised to a half power.

View File

@ -92,13 +92,17 @@ LIRGeneratorX86Shared::lowerModI(MMod *mod)
int32_t rhs = mod->rhs()->toConstant()->value().toInt32();
int32_t shift;
JS_FLOOR_LOG2(shift, rhs);
if (1 << shift == rhs) {
if (rhs > 0 && 1 << shift == rhs) {
LModPowTwoI *lir = new LModPowTwoI(useRegisterAtStart(mod->lhs()), shift);
return assignSnapshot(lir) && defineReuseInput(lir, mod, 0);
if (mod->fallible() && !assignSnapshot(lir))
return false;
return defineReuseInput(lir, mod, 0);
}
}
LModI *lir = new LModI(useRegister(mod->lhs()), useRegister(mod->rhs()), tempFixed(eax));
return assignSnapshot(lir) && defineFixed(lir, mod, LAllocation(AnyRegister(edx)));
if (mod->fallible() && !assignSnapshot(lir))
return false;
return defineFixed(lir, mod, LAllocation(AnyRegister(edx)));
}
bool

View File

@ -0,0 +1,18 @@
var f = function(i,j) { return i%j|0 };
assertEq(f(10, 0), 0);
var f = function(i,j) { return i%j|0 };
assertEq(f(0, 0), 0);
var f = function(i,j) { return i%j|0 };
assertEq(f(-Math.pow(2,31), -1), 0);
var f = function(i,j) { return i%j|0 };
assertEq(f(-4, 4), 0);
var f = function(i) { return i%4|0 };
assertEq(f(-4), 0);
var f = function(i) { return i%4|0 };
assertEq(f(0), 0);
var f = function(i) { return i%3|0 };
assertEq(f(-3), 0);
var f = function(i) { return i%3|0 };
assertEq(f(0), 0);