Bug 1302367 - Regression in Emscripten's conditionals caused by ModD r?nbp,jandem
MozReview-Commit-ID: DotMfdJ8zBD
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -564,16 +564,30 @@ jit::SplitCriticalEdges(MIRGraph& graph)
for (MBasicBlockIterator iter(graph.begin()); iter != graph.end(); iter++) {
MBasicBlock* block = *iter;
if (!SplitCriticalEdgesForBlock(graph, block))
return false;
}
return true;
}
+bool
+jit::IsUint32Type(const MDefinition* def)
+{
+ if (def->isBeta())
+ def = def->getOperand(0);
+
+ if (def->type() != MIRType::Int32)
+ return false;
+
+ return def->isUrsh() && def->getOperand(1)->isConstant() &&
+ def->getOperand(1)->toConstant()->type() == MIRType::Int32 &&
+ def->getOperand(1)->toConstant()->toInt32() == 0;
+}
+
// Return whether a block simply computes the specified constant value.
static bool
BlockComputesConstant(MBasicBlock* block, MDefinition* value, bool* constBool)
{
// Look for values with no uses. This is used to eliminate constant
// computing blocks in condition statements, and the phi which used to
// consume the constant has already been removed.
if (value->hasUses())
--- a/js/src/jit/IonAnalysis.h
+++ b/js/src/jit/IonAnalysis.h
@@ -22,16 +22,19 @@ MOZ_MUST_USE bool
PruneUnusedBranches(MIRGenerator* mir, MIRGraph& graph);
MOZ_MUST_USE bool
FoldTests(MIRGraph& graph);
MOZ_MUST_USE bool
SplitCriticalEdges(MIRGraph& graph);
+bool
+IsUint32Type(const MDefinition* def);
+
enum Observability {
ConservativeObservability,
AggressiveObservability
};
MOZ_MUST_USE bool
EliminatePhis(MIRGenerator* mir, MIRGraph& graph, Observability observe);
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -2734,16 +2734,27 @@ MBinaryBitwiseInstruction::foldsTo(TempA
}
MDefinition*
MBinaryBitwiseInstruction::foldUnnecessaryBitop()
{
if (specialization_ != MIRType::Int32)
return this;
+ // Fold unsigned shift right operator when the second operand is zero and
+ // the only use is an unsigned modulo. Thus, the expression
+ // |(x >>> 0) % y| becomes |x % y|.
+ if (isUrsh() && hasOneDefUse() && IsUint32Type(this)) {
+ for (MUseDefIterator use(this); use; use++) {
+ if (use.def()->isMod() && use.def()->toMod()->isUnsigned())
+ return getOperand(0);
+ break;
+ }
+ }
+
// Eliminate bitwise operations that are no-ops when used on integer
// inputs, such as (x | 0).
MDefinition* lhs = getOperand(0);
MDefinition* rhs = getOperand(1);
if (IsConstant(lhs, 0))
return foldIfZero(0);
@@ -2955,16 +2966,49 @@ NeedNegativeZeroCheck(MDefinition* def)
break;
default:
return true;
}
}
return false;
}
+void
+MBinaryArithInstruction::printOpcode(GenericPrinter& out) const
+{
+ MDefinition::printOpcode(out);
+
+ switch (type()) {
+ case MIRType::Int32:
+ if (isDiv())
+ out.printf(" [%s]", toDiv()->isUnsigned() ? "uint32" : "int32");
+ else if (isMod())
+ out.printf(" [%s]", toMod()->isUnsigned() ? "uint32" : "int32");
+ else
+ out.printf(" [int32]");
+ break;
+ case MIRType::Int64:
+ if (isDiv())
+ out.printf(" [%s]", toDiv()->isUnsigned() ? "uint64" : "int64");
+ else if (isMod())
+ out.printf(" [%s]", toMod()->isUnsigned() ? "uint64" : "int64");
+ else
+ out.printf(" [int64]");
+ break;
+ case MIRType::Float32:
+ out.printf(" [float]");
+ break;
+ case MIRType::Double:
+ out.printf(" [double]");
+ break;
+ default:
+ break;
+ }
+}
+
MBinaryArithInstruction*
MBinaryArithInstruction::New(TempAllocator& alloc, Opcode op,
MDefinition* left, MDefinition* right)
{
switch (op) {
case Op_Add:
return MAdd::New(alloc, left, right);
case Op_Sub:
@@ -4070,18 +4114,27 @@ MToInt32::foldsTo(TempAllocator& alloc)
if (mozilla::NumberEqualsInt32(input->toConstant()->numberToDouble(), &ival))
return MConstant::New(alloc, Int32Value(ival));
break;
default:
break;
}
}
- if (input->type() == MIRType::Int32)
+ // Do not fold the TruncateToInt32 node when the input is uint32 (e.g. ursh
+ // with a zero constant. Consider the test jit-test/tests/ion/bug1247880.js,
+ // where the relevant code is: |(imul(1, x >>> 0) % 2)|. The imul operator
+ // is folded to a MTruncateToInt32 node, which will result in this MIR:
+ // MMod(MTruncateToInt32(MUrsh(x, MConstant(0))), MConstant(2)). Note that
+ // the MUrsh node's type is int32 (since uint32 is not implemented), and
+ // that would fold the MTruncateToInt32 node. This will make the modulo
+ // unsigned, while is should have been signed.
+ if (input->type() == MIRType::Int32 && !IsUint32Type(input))
return input;
+
return this;
}
void
MToInt32::analyzeEdgeCasesBackward()
{
if (!NeedNegativeZeroCheck(this))
setCanBeNegativeZero(false);
@@ -4089,17 +4142,25 @@ MToInt32::analyzeEdgeCasesBackward()
MDefinition*
MTruncateToInt32::foldsTo(TempAllocator& alloc)
{
MDefinition* input = getOperand(0);
if (input->isBox())
input = input->getOperand(0);
- if (input->type() == MIRType::Int32)
+ // Do not fold the TruncateToInt32 node when the input is uint32 (e.g. ursh
+ // with a zero constant. Consider the test jit-test/tests/ion/bug1247880.js,
+ // where the relevant code is: |(imul(1, x >>> 0) % 2)|. The imul operator
+ // is folded to a MTruncateToInt32 node, which will result in this MIR:
+ // MMod(MTruncateToInt32(MUrsh(x, MConstant(0))), MConstant(2)). Note that
+ // the MUrsh node's type is int32 (since uint32 is not implemented), and
+ // that would fold the MTruncateToInt32 node. This will make the modulo
+ // unsigned, while is should have been signed.
+ if (input->type() == MIRType::Int32 && !IsUint32Type(input))
return input;
if (input->type() == MIRType::Double && input->isConstant()) {
int32_t ret = ToInt32(input->toConstant()->toDouble());
return MConstant::New(alloc, Int32Value(ret));
}
return this;
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -6178,16 +6178,17 @@ class MBinaryArithInstruction
MDefinition* left, MDefinition* right);
bool constantDoubleResult(TempAllocator& alloc);
void setMustPreserveNaN(bool b) { mustPreserveNaN_ = b; }
bool mustPreserveNaN() const { return mustPreserveNaN_; }
MDefinition* foldsTo(TempAllocator& alloc) override;
+ void printOpcode(GenericPrinter& out) const override;
virtual double getIdentity() = 0;
void setSpecialization(MIRType type) {
specialization_ = type;
setResultType(type);
}
void setInt32Specialization() {
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -1565,20 +1565,28 @@ MMod::computeRange(TempAllocator& alloc)
return;
// If RHS can be zero, the result can be NaN.
if (rhs.lower() <= 0 && rhs.upper() >= 0)
return;
// If both operands are non-negative integers, we can optimize this to an
// unsigned mod.
- if (specialization() == MIRType::Int32 && lhs.lower() >= 0 && rhs.lower() > 0 &&
- !lhs.canHaveFractionalPart() && !rhs.canHaveFractionalPart())
- {
- unsigned_ = true;
+ if (specialization() == MIRType::Int32 && rhs.lower() > 0) {
+ bool hasDoubles = lhs.lower() < 0 || lhs.canHaveFractionalPart() ||
+ rhs.canHaveFractionalPart();
+ // It is not possible to check that lhs.lower() >= 0, since the range
+ // of a ursh with rhs a 0 constant is wrapped around the int32 range in
+ // Range::Range(). However, IsUint32Type() will only return true for
+ // nodes that lie in the range [0, UINT32_MAX].
+ bool hasUint32s = IsUint32Type(getOperand(0)) &&
+ getOperand(1)->type() == MIRType::Int32 &&
+ (IsUint32Type(getOperand(1)) || getOperand(1)->isConstant());
+ if (!hasDoubles || hasUint32s)
+ unsigned_ = true;
}
// For unsigned mod, we have to convert both operands to unsigned.
// Note that we handled the case of a zero rhs above.
if (unsigned_) {
// The result of an unsigned mod will never be unsigned-greater than
// either operand.
uint32_t lhsBound = Max<uint32_t>(lhs.lower(), lhs.upper());