Attachment 361360 Details for Bug 194222 – WIP

[patch] WIP

WIP_194222 (text/plain), 7.09 KB, created by Robin Morisset on 2019-02-06 18:31:51 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-02-06 18:31:51 PST

Size: 7.09 KB

patch

obsolete

>Index: Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(revision 241108)
>+++ Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(working copy)
>@@ -484,8 +484,106 @@
>     }
>     
> private:
>+    Value* buildInst(Opcode opcode, Value* child0 = nullptr, Value* child1 = nullptr, Value* child2 = nullptr)
>+    {
>+        if (Value* value = tryReducing(opcode, child0, child1, child2))
>+            return value;
>+
>+        if (child1 == nullptr)
>+            return m_insertionSet.insert<Value>(m_index, opcode, m_value->origin(), child0);
>+        if (child2 == nullptr)
>+            return m_insertionSet.insert<Value>(m_index, opcode, m_value->origin(), child0, child1);
>+        return m_insertionSet.insert<Value>(m_index, opcode, m_value->origin(), child0, child1, child2);
>+    }
>+
>+    // This function returns nullptr if it cannot find a clever(er) way of producing the requested value, and the requested value otherwise.
>+    Value* tryReducing(Opcode opcode, Value* child0 = nullptr, Value* child1 = nullptr, Value* child2 = nullptr)
>+    {
>+        (void) child2; // child2 is currently unused as this is WIP only supporting MUL.
>+        switch(opcode) {
>+        case Mul:
>+            if (Value* value = tryReducingCommutativity(opcode, child0, child1))
>+                return value;
>+                
>+            if (Value* value = child0->mulConstant(m_proc, child1))
>+                return value;
>+
>+            if (child1->hasInt()) {
>+                int64_t factor = child1->asInt();
>+
>+                // Turn this: Mul(value, 0)
>+                // Into this: 0
>+                // Note that we don't do this for doubles because that's wrong. For example, -1 * 0
>+                // and 1 * 0 yield different results.
>+                if (!factor)
>+                    return child1;
>+
>+                // Turn this: Mul(value, 1)
>+                // Into this: value
>+                if (factor == 1)
>+                    return child0;
>+
>+                // Turn this: Mul(value, -1)
>+                // Into this: Neg(value)
>+                if (factor == -1)
>+                    return buildInst(Neg, child0);
>+
>+                // Turn this: Mul(value, constant)
>+                // Into this: Shl(value, log2(constant))
>+                if (hasOneBitSet(factor)) {
>+                    unsigned shiftAmount = WTF::fastLog2(static_cast<uint64_t>(factor));
>+                    Value* shiftConstant = m_insertionSet.insert<Const32Value>(m_index, m_value->origin(), shiftAmount);
>+                    return buildInst(Shl, child0, shiftConstant);
>+                }
>+            } else if (child1->hasDouble() && child1->asDouble() == 1.0)
>+                    return child0;
>+
>+            break;
>+        default:
>+            break;
>+        }
>+        return nullptr;
>+    }
>+
>+    // We canonicalize commutative operations in three ways:
>+    // - AtomicStrongCAS first (important for some patterns to be recognized by LowerToAir)
>+    // - Constants last (makes it easier to eliminate them)
>+    // - Other operands sorted by their index (we use the index over the address to make this at least slightly deterministic)
>+    Value* tryReducingCommutativity(Opcode opcode, Value* child0, Value* child1)
>+    {
>+        ASSERT(child0 != nullptr);
>+        ASSERT(child1 != nullptr);
>+
>+        if (child1->isConstant() || child0->opcode() == AtomicStrongCAS)
>+            return nullptr;
>+
>+        if (child0->isConstant() || child1->opcode() == AtomicStrongCAS || child0->index() > child1->index())
>+            return buildInst(opcode, child1, child0);
>+        
>+        return nullptr;
>+    }
>+
>     void reduceValueStrength()
>     {
>+        Value* reducedValue = nullptr;
>+        switch(m_value->numChildren()) {
>+            case 1:
>+                reducedValue = tryReducing(m_value->opcode(), m_value->child(0));
>+                break;
>+            case 2:
>+                reducedValue = tryReducing(m_value->opcode(), m_value->child(0), m_value->child(1));
>+                break;
>+            case 3:
>+                reducedValue = tryReducing(m_value->opcode(), m_value->child(0), m_value->child(1), m_value->child(2));
>+                break;
>+            default:
>+                break;
>+        }
>+        if (reducedValue) {
>+            replaceWithIdentity(reducedValue);
>+            return;
>+        }
>+
>         switch (m_value->opcode()) {
>         case Opaque:
>             // Turn this: Opaque(Opaque(value))
>@@ -636,70 +734,6 @@
>             
>             break;
> 
>-        case Mul:
>-            handleCommutativity();
>-
>-            // Turn this: Mul(constant1, constant2)
>-            // Into this: constant1 * constant2
>-            if (Value* value = m_value->child(0)->mulConstant(m_proc, m_value->child(1))) {
>-                replaceWithNewValue(value);
>-                break;
>-            }
>-
>-            if (m_value->child(1)->hasInt()) {
>-                int64_t factor = m_value->child(1)->asInt();
>-
>-                // Turn this: Mul(value, 0)
>-                // Into this: 0
>-                // Note that we don't do this for doubles because that's wrong. For example, -1 * 0
>-                // and 1 * 0 yield different results.
>-                if (!factor) {
>-                    replaceWithIdentity(m_value->child(1));
>-                    break;
>-                }
>-
>-                // Turn this: Mul(value, 1)
>-                // Into this: value
>-                if (factor == 1) {
>-                    replaceWithIdentity(m_value->child(0));
>-                    break;
>-                }
>-
>-                // Turn this: Mul(value, -1)
>-                // Into this: Sub(0, value)
>-                if (factor == -1) {
>-                    replaceWithNewValue(
>-                        m_proc.add<Value>(
>-                            Sub, m_value->origin(),
>-                            m_insertionSet.insertIntConstant(m_index, m_value, 0),
>-                            m_value->child(0)));
>-                    break;
>-                }
>-                
>-                // Turn this: Mul(value, constant)
>-                // Into this: Shl(value, log2(constant))
>-                if (hasOneBitSet(factor)) {
>-                    unsigned shiftAmount = WTF::fastLog2(static_cast<uint64_t>(factor));
>-                    replaceWithNewValue(
>-                        m_proc.add<Value>(
>-                            Shl, m_value->origin(), m_value->child(0),
>-                            m_insertionSet.insert<Const32Value>(
>-                                m_index, m_value->origin(), shiftAmount)));
>-                    break;
>-                }
>-            } else if (m_value->child(1)->hasDouble()) {
>-                double factor = m_value->child(1)->asDouble();
>-
>-                // Turn this: Mul(value, 1)
>-                // Into this: value
>-                if (factor == 1) {
>-                    replaceWithIdentity(m_value->child(0));
>-                    break;
>-                }
>-            }
>-
>-            break;
>-
>         case Div:
>             // Turn this: Div(constant1, constant2)
>             // Into this: constant1 / constant2

Actions: View | Formatted Diff | Diff

Attachments on bug 194222: 361360 | 361361 | 361452 | 361467 | 361480