Bug 959119: Take into account the use's index when determining whether a mir node can consume a float32; r=sstangl
authorBenjamin Bouvier <benj@benj.me>
Mon, 24 Feb 2014 16:23:50 +0100
changeset 170553 9058ac74ec2839d134036156536e382d142acb3c
parent 170552 d22ac63d67f134fbaf49686ecd5c6754c58078f9
child 170554 bc95fe54b4ee412dbf52962e8175773464fa7939
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerssstangl
bugs959119
milestone30.0a1
Bug 959119: Take into account the use's index when determining whether a mir node can consume a float32; r=sstangl
js/src/jit-test/tests/TypedObject/bug959119.js
js/src/jit/IonAnalysis.cpp
js/src/jit/MIR.cpp
js/src/jit/MIR.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/TypedObject/bug959119.js
@@ -0,0 +1,21 @@
+// This test exposed a bug in float32 optimization.
+// The (inlined and optimized) code for `add()` created
+// MDiv instructions specialized to integers, which was
+// then "respecialized" to float32, leading to internal
+// assertion errors.
+//
+// Public domain.
+
+if (!this.hasOwnProperty("TypedObject"))
+  quit();
+
+var {StructType,uint8,float32} = TypedObject;
+var RgbColor2 = new StructType({r: uint8, g: float32, b: uint8});
+RgbColor2.prototype.add = function(c) {
+  this.g += c;
+  this.b += c;
+};
+var gray = new RgbColor2({r: 129, g: 128, b: 127});
+gray.add(1);
+gray.add(2);
+
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -794,48 +794,48 @@ TypeAnalyzer::markPhiConsumers()
         if (mir->shouldCancel("Ensure Float32 commutativity - Consumer Phis - Initial state"))
             return false;
 
         for (MPhiIterator phi(block->phisBegin()); phi != block->phisEnd(); ++phi) {
             JS_ASSERT(!phi->isInWorklist());
             bool canConsumeFloat32 = true;
             for (MUseDefIterator use(*phi); canConsumeFloat32 && use; use++) {
                 MDefinition *usedef = use.def();
-                canConsumeFloat32 &= usedef->isPhi() || usedef->canConsumeFloat32();
+                canConsumeFloat32 &= usedef->isPhi() || usedef->canConsumeFloat32(use.use());
             }
             phi->setCanConsumeFloat32(canConsumeFloat32);
             if (canConsumeFloat32 && !addPhiToWorklist(*phi))
                 return false;
         }
     }
 
     while (!phiWorklist_.empty()) {
         if (mir->shouldCancel("Ensure Float32 commutativity - Consumer Phis - Fixed point"))
             return false;
 
         MPhi *phi = popPhi();
-        JS_ASSERT(phi->canConsumeFloat32());
+        JS_ASSERT(phi->canConsumeFloat32(nullptr /* unused */));
 
         bool validConsumer = true;
         for (MUseDefIterator use(phi); use; use++) {
             MDefinition *def = use.def();
-            if (def->isPhi() && !def->canConsumeFloat32()) {
+            if (def->isPhi() && !def->canConsumeFloat32(use.use())) {
                 validConsumer = false;
                 break;
             }
         }
 
         if (validConsumer)
             continue;
 
         // Propagate invalidated phis
         phi->setCanConsumeFloat32(false);
         for (size_t i = 0, e = phi->numOperands(); i < e; ++i) {
             MDefinition *input = phi->getOperand(i);
-            if (input->isPhi() && !input->isInWorklist() && input->canConsumeFloat32())
+            if (input->isPhi() && !input->isInWorklist() && input->canConsumeFloat32(nullptr /* unused */))
             {
                 if (!addPhiToWorklist(input->toPhi()))
                     return false;
             }
         }
     }
     return true;
 }
@@ -971,17 +971,17 @@ TypeAnalyzer::checkFloatCoherency()
             return false;
 
         for (MDefinitionIterator def(*block); def; def++) {
             if (def->type() != MIRType_Float32)
                 continue;
 
             for (MUseDefIterator use(*def); use; use++) {
                 MDefinition *consumer = use.def();
-                JS_ASSERT(consumer->isConsistentFloat32Use());
+                JS_ASSERT(consumer->isConsistentFloat32Use(use.use()));
             }
         }
     }
 #endif
     return true;
 }
 
 bool
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -38,17 +38,17 @@ ConvertDefinitionToDouble(TempAllocator 
     consumer->block()->insertBefore(consumer, replace);
 }
 
 static bool
 CheckUsesAreFloat32Consumers(MInstruction *ins)
 {
     bool allConsumerUses = true;
     for (MUseDefIterator use(ins); allConsumerUses && use; use++)
-        allConsumerUses &= use.def()->canConsumeFloat32();
+        allConsumerUses &= use.def()->canConsumeFloat32(use.use());
     return allConsumerUses;
 }
 
 void
 MDefinition::PrintOpcodeName(FILE *fp, MDefinition::Opcode op)
 {
     static const char * const names[] =
     {
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -465,22 +465,22 @@ class MDefinition : public MNode
 
         return !resultTypeSet() || resultTypeSet()->mightBeType(ValueTypeFromMIRType(type));
     }
 
     // Float32 specialization operations (see big comment in IonAnalysis before the Float32
     // specialization algorithm).
     virtual bool isFloat32Commutative() const { return false; }
     virtual bool canProduceFloat32() const { return false; }
-    virtual bool canConsumeFloat32() const { return false; }
+    virtual bool canConsumeFloat32(MUse *use) const { return false; }
     virtual void trySpecializeFloat32(TempAllocator &alloc) {}
 #ifdef DEBUG
     // Used during the pass that checks that Float32 flow into valid MDefinitions
-    virtual bool isConsistentFloat32Use() const {
-        return type() == MIRType_Float32 || canConsumeFloat32();
+    virtual bool isConsistentFloat32Use(MUse *use) const {
+        return type() == MIRType_Float32 || canConsumeFloat32(use);
     }
 #endif
 
     // Returns the beginning of this definition's use chain.
     MUseIterator usesBegin() const {
         return uses_.begin();
     }
 
@@ -1324,17 +1324,17 @@ class MTest
 
     void markOperandCantEmulateUndefined() {
         operandMightEmulateUndefined_ = false;
     }
     bool operandMightEmulateUndefined() const {
         return operandMightEmulateUndefined_;
     }
 #ifdef DEBUG
-    bool isConsistentFloat32Use() const {
+    bool isConsistentFloat32Use(MUse *use) const {
         return true;
     }
 #endif
 };
 
 // Returns from this function to the previous caller.
 class MReturn
   : public MAryControlInstruction<1, 0>,
@@ -2055,17 +2055,17 @@ class MAssertFloat32 : public MUnaryInst
 
   public:
     INSTRUCTION_HEADER(AssertFloat32)
 
     static MAssertFloat32 *New(TempAllocator &alloc, MDefinition *value, bool mustBeFloat32) {
         return new(alloc) MAssertFloat32(value, mustBeFloat32);
     }
 
-    bool canConsumeFloat32() const { return true; }
+    bool canConsumeFloat32(MUse *use) const { return true; }
 
     bool mustBeFloat32() const { return mustBeFloat32_; }
 };
 
 class MGetDynamicName
   : public MAryInstruction<2>,
     public MixPolicy<ObjectPolicy<0>, ConvertToStringPolicy<1> >
 {
@@ -2331,17 +2331,18 @@ class MCompare
     void collectRangeInfoPreTrunc();
 
     void trySpecializeFloat32(TempAllocator &alloc);
     bool isFloat32Commutative() const { return true; }
     bool truncate();
     bool isOperandTruncated(size_t index) const;
 
 # ifdef DEBUG
-    bool isConsistentFloat32Use() const {
+    bool isConsistentFloat32Use(MUse *use) const {
+        // Both sides of the compare can be Float32
         return compareType_ == Compare_Float32;
     }
 # endif
 
   protected:
     bool congruentTo(MDefinition *ins) const {
         if (!MBinaryInstruction::congruentTo(ins))
             return false;
@@ -2897,17 +2898,17 @@ class MToDouble
         return AliasSet::None();
     }
 
     void computeRange(TempAllocator &alloc);
     bool truncate();
     bool isOperandTruncated(size_t index) const;
 
 #ifdef DEBUG
-    bool isConsistentFloat32Use() const { return true; }
+    bool isConsistentFloat32Use(MUse *use) const { return true; }
 #endif
 };
 
 // Converts a primitive (either typed or untyped) to a float32. If the input is
 // not primitive at runtime, a bailout occurs.
 class MToFloat32
   : public MUnaryInstruction,
     public ToDoublePolicy
@@ -2956,17 +2957,17 @@ class MToFloat32
         return congruentIfOperandsEqual(ins);
     }
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 
     void computeRange(TempAllocator &alloc);
 
-    bool canConsumeFloat32() const { return true; }
+    bool canConsumeFloat32(MUse *use) const { return true; }
     bool canProduceFloat32() const { return true; }
 };
 
 // Converts a uint32 to a double (coming from asm.js).
 class MAsmJSUnsignedToDouble
   : public MUnaryInstruction
 {
     MAsmJSUnsignedToDouble(MDefinition *def)
@@ -3072,17 +3073,17 @@ class MToInt32
     }
 
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
     void computeRange(TempAllocator &alloc);
 
 #ifdef DEBUG
-    bool isConsistentFloat32Use() const { return true; }
+    bool isConsistentFloat32Use(MUse *use) const { return true; }
 #endif
 };
 
 // Converts a value or typed input to a truncated int32, for use with bitwise
 // operations. This is an infallible ValueToECMAInt32.
 class MTruncateToInt32 : public MUnaryInstruction
 {
     MTruncateToInt32(MDefinition *def)
@@ -3108,17 +3109,17 @@ class MTruncateToInt32 : public MUnaryIn
     }
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 
     void computeRange(TempAllocator &alloc);
     bool isOperandTruncated(size_t index) const;
 # ifdef DEBUG
-    bool isConsistentFloat32Use() const {
+    bool isConsistentFloat32Use(MUse *use) const {
         return true;
     }
 #endif
 };
 
 // Converts any type to a string
 class MToString : public MUnaryInstruction
 {
@@ -4569,17 +4570,17 @@ class MPhi MOZ_FINAL : public MDefinitio
     bool canProduceFloat32() const {
         return canProduceFloat32_;
     }
 
     void setCanProduceFloat32(bool can) {
         canProduceFloat32_ = can;
     }
 
-    bool canConsumeFloat32() const {
+    bool canConsumeFloat32(MUse *use) const {
         return canConsumeFloat32_;
     }
 
     void setCanConsumeFloat32(bool can) {
         canConsumeFloat32_ = can;
     }
 };
 
@@ -5661,17 +5662,17 @@ class MNot
     TypePolicy *typePolicy() {
         return this;
     }
     void collectRangeInfoPreTrunc();
 
     void trySpecializeFloat32(TempAllocator &alloc);
     bool isFloat32Commutative() const { return true; }
 #ifdef DEBUG
-    bool isConsistentFloat32Use() const {
+    bool isConsistentFloat32Use(MUse *use) const {
         return true;
     }
 #endif
 };
 
 // Bailout if index + minimum < 0 or index + maximum >= length. The length used
 // in a bounds check must not be negative, or the wrong result may be computed
 // (unsigned comparisons may be used).
@@ -6353,17 +6354,19 @@ class MStoreTypedArrayElement
     bool racy() const {
         return racy_;
     }
     void setRacy() {
         racy_ = true;
     }
     bool isOperandTruncated(size_t index) const;
 
-    bool canConsumeFloat32() const { return arrayType_ == ScalarTypeDescr::TYPE_FLOAT32; }
+    bool canConsumeFloat32(MUse *use) const {
+        return use->index() == 2 && arrayType_ == ScalarTypeDescr::TYPE_FLOAT32;
+    }
 };
 
 class MStoreTypedArrayElementHole
   : public MAryInstruction<4>,
     public StoreTypedArrayHolePolicy
 {
     int arrayType_;
 
@@ -6419,17 +6422,19 @@ class MStoreTypedArrayElementHole
     MDefinition *value() const {
         return getOperand(3);
     }
     AliasSet getAliasSet() const {
         return AliasSet::Store(AliasSet::TypedArrayElement);
     }
     bool isOperandTruncated(size_t index) const;
 
-    bool canConsumeFloat32() const { return arrayType_ == ScalarTypeDescr::TYPE_FLOAT32; }
+    bool canConsumeFloat32(MUse *use) const {
+        return use->index() == 3 && arrayType_ == ScalarTypeDescr::TYPE_FLOAT32;
+    }
 };
 
 // Store a value infallibly to a statically known typed array.
 class MStoreTypedArrayElementStatic :
     public MBinaryInstruction
   , public StoreTypedArrayElementStaticPolicy
 {
     MStoreTypedArrayElementStatic(TypedArrayObject *typedArray, MDefinition *ptr, MDefinition *v)
@@ -6464,17 +6469,19 @@ class MStoreTypedArrayElementStatic :
 
     MDefinition *ptr() const { return getOperand(0); }
     MDefinition *value() const { return getOperand(1); }
     AliasSet getAliasSet() const {
         return AliasSet::Store(AliasSet::TypedArrayElement);
     }
     bool isOperandTruncated(size_t index) const;
 
-    bool canConsumeFloat32() const { return typedArray_->type() == ScalarTypeDescr::TYPE_FLOAT32; }
+    bool canConsumeFloat32(MUse *use) const {
+        return use->index() == 1 && typedArray_->type() == ScalarTypeDescr::TYPE_FLOAT32;
+    }
 };
 
 // Compute an "effective address", i.e., a compound computation of the form:
 //   base + index * scale + displacement
 class MEffectiveAddress : public MBinaryInstruction
 {
     MEffectiveAddress(MDefinition *base, MDefinition *index, Scale scale, int32_t displacement)
       : MBinaryInstruction(base, index), scale_(scale), displacement_(displacement)
@@ -7852,17 +7859,17 @@ class MSetElementCache
     bool guardHoles() const {
         return guardHoles_;
     }
 
     TypePolicy *typePolicy() {
         return this;
     }
 
-    bool canConsumeFloat32() const { return true; }
+    bool canConsumeFloat32(MUse *use) const { return use->index() == 2; }
 };
 
 class MCallGetProperty
   : public MUnaryInstruction,
     public BoxInputsPolicy
 {
     CompilerRootPropertyName name_;
     bool idempotent_;
@@ -8238,17 +8245,17 @@ class MFloor
     TypePolicy *typePolicy() {
         return this;
     }
     bool isFloat32Commutative() const {
         return true;
     }
     void trySpecializeFloat32(TempAllocator &alloc);
 #ifdef DEBUG
-    bool isConsistentFloat32Use() const {
+    bool isConsistentFloat32Use(MUse *use) const {
         return true;
     }
 #endif
 };
 
 // Inlined version of Math.round().
 class MRound
   : public MUnaryInstruction,
@@ -8883,20 +8890,20 @@ class MPostWriteBarrier
 
     bool hasValue() const {
         return hasValue_;
     }
     MDefinition *value() const {
         return getOperand(1);
     }
 #ifdef DEBUG
-    bool isConsistentFloat32Use() const {
+    bool isConsistentFloat32Use(MUse *use) const {
         // During lowering, values that neither have object nor value MIR type
         // are ignored, thus Float32 can show up at this point without any issue.
-        return true;
+        return use->index() == 1;
     }
 #endif
 };
 
 class MNewSlots : public MNullaryInstruction
 {
     unsigned nslots_;