[webkit-changes] [279473] trunk/Source/JavaScriptCore

Thu, 01 Jul 2021 11:25:04 -0700

Title: [279473] trunk/Source/_javascript_Core
Revision
279473
Author
commit-qu...@webkit.org
Date
2021-07-01 11:24:09 -0700 (Thu, 01 Jul 2021)

Log Message

Remove unnecessary canBeInternal invocations to mitigate the cost of potential unmatched patterns in B3LowerToAir
https://bugs.webkit.org/show_bug.cgi?id=227508

Patch by Yijia Huang <yijia_hu...@apple.com> on 2021-07-01
Reviewed by Filip Pizlo.

The bit pattern doesn't cause worse code generation in the all-internals-are-captured
case. So, they don't need canBeInternal checks which might terminate potential matched
scenarios.

The equivalent pattern of SBFIZ is ((src << amount) >> amount) << lsb. Given the code:

a = x << C
b = a >> C
c = b << D

print(a)
print(b)
print(c)

The pattern won't match because of !canBeInternal for a and b (useCounts > 1).
So, this would emit three separate instructions. But if we removed canBeInternal,
it would still be just three separate instructions, and they wouldn't be any more
expensive. Suppose the print(b) is removed, above. Then, with the canBeInternal check,
it is emitting three instructions. Without the canBeInternal check, it would emit only
two (x << C and SBFIZ to compute c). And that would be less expensive.

* b3/B3LowerToAir.cpp:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (279472 => 279473)


--- trunk/Source/_javascript_Core/ChangeLog	2021-07-01 17:41:51 UTC (rev 279472)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-07-01 18:24:09 UTC (rev 279473)
@@ -1,5 +1,35 @@
 2021-07-01  Yijia Huang  <yijia_hu...@apple.com>
 
+        Remove unnecessary canBeInternal invocations to mitigate the cost of potential unmatched patterns in B3LowerToAir
+        https://bugs.webkit.org/show_bug.cgi?id=227508
+
+        Reviewed by Filip Pizlo.
+
+        The bit pattern doesn't cause worse code generation in the all-internals-are-captured 
+        case. So, they don't need canBeInternal checks which might terminate potential matched 
+        scenarios.
+
+        The equivalent pattern of SBFIZ is ((src << amount) >> amount) << lsb. Given the code:
+
+        a = x << C
+        b = a >> C
+        c = b << D
+
+        print(a)
+        print(b)
+        print(c)
+
+        The pattern won't match because of !canBeInternal for a and b (useCounts > 1). 
+        So, this would emit three separate instructions. But if we removed canBeInternal, 
+        it would still be just three separate instructions, and they wouldn't be any more 
+        expensive. Suppose the print(b) is removed, above. Then, with the canBeInternal check, 
+        it is emitting three instructions. Without the canBeInternal check, it would emit only 
+        two (x << C and SBFIZ to compute c). And that would be less expensive.
+
+        * b3/B3LowerToAir.cpp:
+
+2021-07-01  Yijia Huang  <yijia_hu...@apple.com>
+
         Add a new pattern to instruction selector to use EXTR supported by ARM64
         https://bugs.webkit.org/show_bug.cgi?id=227171

Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (279472 => 279473)


--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp	2021-07-01 17:41:51 UTC (rev 279472)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp	2021-07-01 18:24:09 UTC (rev 279473)
@@ -2769,7 +2769,7 @@
                 Air::Opcode opcode = opcodeForType(ExtractUnsignedBitfield32, ExtractUnsignedBitfield64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp)) 
                     return false;
-                if (left->opcode() != ZShr || !canBeInternal(left))
+                if (left->opcode() != ZShr)
                     return false;
 
                 Value* srcValue = left->child(0);
@@ -2787,7 +2787,6 @@
                     return false;
 
                 append(opcode, tmp(srcValue), imm(lsbValue), imm(width), tmp(m_value));
-                commitInternal(left);
                 return true;
             };
 
@@ -2799,7 +2798,7 @@
                 Air::Opcode opcode = opcodeForType(ClearBitsWithMask32, ClearBitsWithMask64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Tmp)) 
                     return false;
-                if (right->opcode() != BitXor || !canBeInternal(right))
+                if (right->opcode() != BitXor)
                     return false;
 
                 Value* nValue = left;
@@ -2809,7 +2808,6 @@
                     return false;
 
                 append(opcode, tmp(nValue), tmp(mValue), tmp(m_value));
-                commitInternal(right);
                 return true;
             };
 
@@ -2872,12 +2870,8 @@
                 Air::Opcode opcode = opcodeForType(InsertBitField32, InsertBitField64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp)) 
                     return false;
-                if (left->opcode() != Shl || !canBeInternal(left))
+                if (left->opcode() != Shl || right->opcode() != BitAnd || left->child(0)->opcode() != BitAnd)
                     return false;
-                if (right->opcode() != BitAnd || !canBeInternal(right))
-                    return false;
-                if (left->child(0)->opcode() != BitAnd || !canBeInternal(left->child(0)))
-                    return false;
 
                 Value* nValue = left->child(0)->child(0);
                 Value* maskValue1 = left->child(0)->child(1);
@@ -2913,9 +2907,6 @@
                 Tmp result = tmp(m_value);
                 append(relaxedMoveForType(m_value->type()), tmp(dValue), result);
                 append(opcode, tmp(nValue), imm(lsbValue), imm(width), result);
-                commitInternal(left->child(0));
-                commitInternal(left);
-                commitInternal(right);
                 return true;
             };
 
@@ -2929,12 +2920,8 @@
                 Air::Opcode opcode = opcodeForType(ExtractInsertBitfieldAtLowEnd32, ExtractInsertBitfieldAtLowEnd64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp)) 
                     return false;
-                if (left->opcode() != BitAnd || !canBeInternal(left))
+                if (left->opcode() != BitAnd || left->child(0)->opcode() != ZShr || right->opcode() != BitAnd)
                     return false;
-                if (left->child(0)->opcode() != ZShr || !canBeInternal(left->child(0)))
-                    return false;
-                if (right->opcode() != BitAnd || !canBeInternal(right))
-                    return false;
 
                 Value* nValue = left->child(0)->child(0);
                 Value* maskValue1 = left->child(1);
@@ -2969,9 +2956,6 @@
                 Tmp result = tmp(m_value);
                 append(relaxedMoveForType(m_value->type()), tmp(dValue), result);
                 append(opcode, tmp(nValue), imm(lsbValue), imm(width), result);
-                commitInternal(left->child(0));
-                commitInternal(left);
-                commitInternal(right);
                 return true;
             };
 
@@ -2983,7 +2967,7 @@
                 Air::Opcode opcode = opcodeForType(OrNot32, OrNot64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Tmp)) 
                     return false;
-                if (right->opcode() != BitXor || !canBeInternal(right))
+                if (right->opcode() != BitXor)
                     return false;
 
                 Value* nValue = left;
@@ -2993,7 +2977,6 @@
                     return false;
 
                 append(opcode, tmp(nValue), tmp(mValue), tmp(m_value));
-                commitInternal(right);
                 return true;
             };
 
@@ -3045,7 +3028,7 @@
                 Air::Opcode opcode = opcodeForType(InsertUnsignedBitfieldInZero32, InsertUnsignedBitfieldInZero64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp))
                     return false;
-                if (left->opcode() != BitAnd || !canBeInternal(left))
+                if (left->opcode() != BitAnd)
                     return false;
 
                 Value* nValue = left->child(0);
@@ -3063,7 +3046,6 @@
                     return false;
 
                 append(opcode, tmp(nValue), imm(right), imm(width), tmp(m_value));
-                commitInternal(left);
                 return true;
             };
 
@@ -3078,10 +3060,8 @@
                 Air::Opcode opcode = opcodeForType(InsertSignedBitfieldInZero32, InsertSignedBitfieldInZero64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp))
                     return false;
-                if (left->opcode() != SShr || !canBeInternal(left))
+                if (left->opcode() != SShr || left->child(0)->opcode() != Shl)
                     return false;
-                if (left->child(0)->opcode() != Shl || !canBeInternal(left->child(0)))
-                    return false;
 
                 Value* srcValue = left->child(0)->child(0);
                 Value* amount1Value = left->child(0)->child(1);
@@ -3103,8 +3083,6 @@
                     return false;
 
                 append(opcode, tmp(srcValue), imm(lsbValue), imm(width), tmp(m_value));
-                commitInternal(left->child(0));
-                commitInternal(left);
                 return true;
             };
 
@@ -3130,10 +3108,8 @@
                 Air::Opcode opcode = opcodeForType(ExtractSignedBitfield32, ExtractSignedBitfield64, m_value->type());
                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp))
                     return false;
-                if (left->opcode() != Shl || !canBeInternal(left))
+                if (left->opcode() != Shl || (left->child(0)->opcode() != ZShr && left->child(0)->opcode() != SShr))
                     return false;
-                if ((left->child(0)->opcode() != ZShr && left->child(0)->opcode() != SShr) || !canBeInternal(left->child(0)))
-                    return false;
 
                 Value* srcValue = left->child(0)->child(0);
                 Value* lsbValue = left->child(0)->child(1);
@@ -3155,8 +3131,6 @@
                     return false;
 
                 append(opcode, tmp(srcValue), imm(lsbValue), imm(width), tmp(m_value));
-                commitInternal(left->child(0));
-                commitInternal(left);
                 return true;
             };
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to