Bug 1137573 - OdinMonkey: Generalize alignment analysis to handle adds with multiple uses r=luke a=ryanvm
authorDan Gohman <sunfish@mozilla.com>
Fri, 27 Feb 2015 11:02:59 -0800
changeset 231192 4c9ebb3591c45ab0d70d7709e7baf960585e844d
parent 231191 5def1d193a0c6f6a8b0ae175ef2be25b46ee248a
child 231193 010198e03e66372608f8463cd4c01a3d9c23e02a
push id28348
push userkwierso@gmail.com
push dateMon, 02 Mar 2015 20:13:43 +0000
treeherdermozilla-central@abb7f0d180da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke, ryanvm
bugs1137573
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1137573 - OdinMonkey: Generalize alignment analysis to handle adds with multiple uses r=luke a=ryanvm
js/src/jit/AlignmentMaskAnalysis.cpp
--- a/js/src/jit/AlignmentMaskAnalysis.cpp
+++ b/js/src/jit/AlignmentMaskAnalysis.cpp
@@ -16,64 +16,64 @@ IsAlignmentMask(uint32_t m)
 {
     // Test whether m is just leading ones and trailing zeros.
     return (-m & ~m) == 0;
 }
 
 static void
 AnalyzeAsmHeapAddress(MDefinition *ptr, MIRGraph &graph)
 {
-    // Fold (a+i)&m to (a&m)+i, since the users of the BitAnd include heap
-    // accesses. This will expose the redundancy for GVN when expressions
-    // like this:
+    // Fold (a+i)&m to (a&m)+i, provided that this doesn't change the result,
+    // since the users of the BitAnd include heap accesses. This will expose
+    // the redundancy for GVN when expressions like this:
     //   a&m
     //   (a+1)&m,
     //   (a+2)&m,
     // are transformed into this:
     //   a&m
     //   (a&m)+1
     //   (a&m)+2
     // and it will allow the constants to be folded by the
     // EffectiveAddressAnalysis pass.
+    //
+    // Putting the add on the outside might seem like it exposes other users of
+    // the expression to the possibility of i32 overflow, if we aren't in asm.js
+    // and they aren't naturally truncating. However, since we use MAdd::NewAsmJS
+    // with MIRType_Int32, we make sure that the value is truncated, just as it
+    // would be by the MBitAnd.
 
     if (!ptr->isBitAnd())
         return;
 
     MDefinition *lhs = ptr->toBitAnd()->getOperand(0);
     MDefinition *rhs = ptr->toBitAnd()->getOperand(1);
-    int lhsIndex = 0;
-    if (lhs->isConstantValue()) {
+    if (lhs->isConstantValue())
         mozilla::Swap(lhs, rhs);
-        lhsIndex = 1;
-    }
-    if (!lhs->isAdd() || !lhs->hasOneUse() || !rhs->isConstantValue())
+    if (!lhs->isAdd() || !rhs->isConstantValue())
         return;
 
     MDefinition *op0 = lhs->toAdd()->getOperand(0);
     MDefinition *op1 = lhs->toAdd()->getOperand(1);
-    int op0Index = 0;
-    if (op0->isConstantValue()) {
+    if (op0->isConstantValue())
         mozilla::Swap(op0, op1);
-        op0Index = 1;
-    }
     if (!op1->isConstantValue())
         return;
 
     uint32_t i = op1->constantValue().toInt32();
     uint32_t m = rhs->constantValue().toInt32();
-    if (!IsAlignmentMask(m) || ((i & m) != i))
+    if (!IsAlignmentMask(m) || (i & m) != i)
         return;
 
-    ptr->replaceAllUsesWith(lhs);
-    ptr->toBitAnd()->replaceOperand(lhsIndex, op0);
-    lhs->toAdd()->replaceOperand(op0Index, ptr);
-
-    MInstructionIterator iter = ptr->block()->begin(ptr->toBitAnd());
-    ++iter;
-    lhs->block()->moveBefore(*iter, lhs->toAdd());
+    // The pattern was matched! Produce the replacement expression.
+    MInstruction *and_ = MBitAnd::NewAsmJS(graph.alloc(), op0, rhs);
+    ptr->block()->insertBefore(ptr->toBitAnd(), and_);
+    MInstruction *add = MAdd::NewAsmJS(graph.alloc(), and_, op1, MIRType_Int32);
+    ptr->block()->insertBefore(ptr->toBitAnd(), add);
+    ptr->replaceAllUsesWith(add);
+    ptr->block()->discard(ptr->toBitAnd());
 }
 
 bool
 AlignmentMaskAnalysis::analyze()
 {
     for (ReversePostorderIterator block(graph_.rpoBegin()); block != graph_.rpoEnd(); block++) {
         for (MInstructionIterator i = block->begin(); i != block->end(); i++) {
             // Note that we don't check for MAsmJSCompareExchangeHeap