Changeset 278390 in webkit

Timestamp:

Jun 2, 2021, 6:07:45 PM (5 years ago)

Author:

rmorisset@apple.com

Message:

B3MoveConstants should filter directly on Values, and only create ValueKeys when useful
​https://bugs.webkit.org/show_bug.cgi?id=226420

Reviewed by Phil Pizlo.

I did a few runs of JetStream2 to measure results, the time spent in B3MoveConstants goes from 160-180ms to 100-110ms.
The total time spent in B3+Air is in the 6 to 8s range, so this is approximately a 1% speedup.

• b3/B3MoveConstants.cpp:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• b3/B3MoveConstants.cpp (modified) (11 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-06-02  Robin Morisset  <rmorisset@apple.com>
+
+        B3MoveConstants should filter directly on Values, and only create ValueKeys when useful
+        https://bugs.webkit.org/show_bug.cgi?id=226420
+
+        Reviewed by Phil Pizlo.
+
+        I did a few runs of JetStream2 to measure results, the time spent in B3MoveConstants goes from 160-180ms to 100-110ms.
+        The total time spent in B3+Air is in the 6 to 8s range, so this is approximately a 1% speedup.
+
+        * b3/B3MoveConstants.cpp:
+
 2021-06-02  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/b3/B3MoveConstants.cpp

@@ -52 +52 @@
     {
         hoistConstants(
-            [&] (const ValueKey& key) -> bool {
-                return key.opcode() == ConstFloat || key.opcode() == ConstDouble;
+            [&] (const Value* value) -> bool {
+                return value->opcode() == ConstFloat || value->opcode() == ConstDouble;
             });
 
@@ -59 +59 @@
         
         hoistConstants(
-            [&] (const ValueKey& key) -> bool {
-                return key.opcode() == Const32 || key.opcode() == Const64 || key.opcode() == ArgumentReg;
+            [&] (const Value* value) -> bool {
+                return value->opcode() == Const32 || value->opcode() == Const64 || value->opcode() == ArgumentReg;
             });
     }
@@ -76 +76 @@
             for (Value* value : *block) {
                 for (Value*& child : value->children()) {
+                    if (!filter(child))
+                        continue;
+
                     ValueKey key = child->key();
-                    if (!filter(key))
-                        continue;
-
                     auto result = valueForConstant.add(key, child);
                     if (result.isNewEntry) {
@@ -116 +116 @@
         for (BasicBlock* block : m_proc) {
             for (Value*& value : *block) {
+                if (!filter(value))
+                    continue;
+
                 ValueKey key = value->key();
-                if (!filter(key))
-                    continue;
-
                 if (valueForConstant.get(key) == value)
                     value = m_proc.add<Value>(Nop, value->origin());
@@ -154 +154 @@
                 // haven't inserted it yet. This inserts the constant if necessary.
                 auto materialize = [&] (Value* child) {
-                    ValueKey key = child->key();
-                    if (!filter(key))
+                    if (!filter(child))
                         return;
 
                     // If we encounter a fast constant, then it must be canonical, since we already
                     // got rid of the non-canonical ones.
-                    ASSERT(valueForConstant.get(key) == child);
+                    ASSERT(valueForConstant.get(child->key()) == child);
 
                     if (child->owner != block) {
@@ -176 +175 @@
                 if (MemoryValue* memoryValue = value->as<MemoryValue>()) {
                     Value* pointer = memoryValue->lastChild();
-                    if (pointer->hasIntPtr() && filter(pointer->key())) {
+                    if (pointer->hasIntPtr() && filter(pointer)) {
                         auto desiredOffset = [&] (Value* otherPointer) -> intptr_t {
                             // We would turn this:
@@ -214 +213 @@
                     case Sub: {
                         Value* addend = value->child(1);
-                        if (!addend->hasInt() || !filter(addend->key()))
+                        if (!addend->hasInt() || !filter(addend))
                             break;
                         int64_t addendConst = addend->asInt();
@@ -262 +261 @@
     {
         for (Value* value : m_proc.values()) {
+            if (!goesInTable(value))
+                continue;
             ValueKey key = value->key();
-            if (goesInTable(key))
-                m_constTable.add(key, m_constTable.size());
+            m_constTable.add(key, m_constTable.size());
         }
         
@@ -283 +283 @@
                     
                     Value*& child = value->child(childIndex);
+                    if (!goesInTable(child))
+                        continue;
+
                     ValueKey key = child->key();
-                    if (!goesInTable(key))
-                        continue;
-
                     child = m_insertionSet.insertValue(
                         valueIndex, key.materialize(m_proc, value->origin()));
@@ -299 +299 @@
             for (unsigned valueIndex = 0; valueIndex < block->size(); ++valueIndex) {
                 Value* value = block->at(valueIndex);
+                if (!goesInTable(value))
+                    continue;
+                if (offLimits.contains(value))
+                    continue;
+
                 ValueKey key = value->key();
-                if (!goesInTable(key))
-                    continue;
-                if (offLimits.contains(value))
-                    continue;
-
                 auto offset = sizeof(int64_t) * m_constTable.get(key);
                 if (!isRepresentableAs<Value::OffsetType>(offset))
@@ -322 +322 @@
     }
 
-    bool goesInTable(const ValueKey& key)
-    {
-        return (key.opcode() == ConstDouble && key != doubleZero())
-            || (key.opcode() == ConstFloat && key != floatZero());
-    }
-
-    static ValueKey doubleZero()
-    {
-        return ValueKey(ConstDouble, Double, 0.0);
-    }
-
-    static ValueKey floatZero()
-    {
-        return ValueKey(ConstFloat, Float, 0.0);
+    bool goesInTable(const Value* value)
+    {
+        switch (value->opcode()) {
+        case ConstDouble: {
+            double doubleZero = 0.0;
+            return bitwise_cast<uint64_t>(value->asDouble()) != bitwise_cast<uint64_t>(doubleZero);
+        }
+        case ConstFloat: {
+            float floatZero = 0.0;
+            return bitwise_cast<uint32_t>(value->asFloat()) != bitwise_cast<uint32_t>(floatZero);
+        }
+        default:
+            break;
+        }
+        return false;
     }