<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[279473] trunk/Source/JavaScriptCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/279473">279473</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2021-07-01 11:24:09 -0700 (Thu, 01 Jul 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>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_huang@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:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreb3B3LowerToAircpp">trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (279472 => 279473)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog    2021-07-01 17:41:51 UTC (rev 279472)
+++ trunk/Source/JavaScriptCore/ChangeLog       2021-07-01 18:24:09 UTC (rev 279473)
</span><span class="lines">@@ -1,5 +1,35 @@
</span><span class="cx"> 2021-07-01  Yijia Huang  <yijia_huang@apple.com>
</span><span class="cx"> 
</span><ins>+        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_huang@apple.com>
+
</ins><span class="cx">         Add a new pattern to instruction selector to use EXTR supported by ARM64
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=227171
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreb3B3LowerToAircpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp (279472 => 279473)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp  2021-07-01 17:41:51 UTC (rev 279472)
+++ trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp     2021-07-01 18:24:09 UTC (rev 279473)
</span><span class="lines">@@ -2769,7 +2769,7 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(ExtractUnsignedBitfield32, ExtractUnsignedBitfield64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp)) 
</span><span class="cx">                     return false;
</span><del>-                if (left->opcode() != ZShr || !canBeInternal(left))
</del><ins>+                if (left->opcode() != ZShr)
</ins><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 Value* srcValue = left->child(0);
</span><span class="lines">@@ -2787,7 +2787,6 @@
</span><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 append(opcode, tmp(srcValue), imm(lsbValue), imm(width), tmp(m_value));
</span><del>-                commitInternal(left);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -2799,7 +2798,7 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(ClearBitsWithMask32, ClearBitsWithMask64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Tmp)) 
</span><span class="cx">                     return false;
</span><del>-                if (right->opcode() != BitXor || !canBeInternal(right))
</del><ins>+                if (right->opcode() != BitXor)
</ins><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 Value* nValue = left;
</span><span class="lines">@@ -2809,7 +2808,6 @@
</span><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 append(opcode, tmp(nValue), tmp(mValue), tmp(m_value));
</span><del>-                commitInternal(right);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -2872,12 +2870,8 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(InsertBitField32, InsertBitField64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp)) 
</span><span class="cx">                     return false;
</span><del>-                if (left->opcode() != Shl || !canBeInternal(left))
</del><ins>+                if (left->opcode() != Shl || right->opcode() != BitAnd || left->child(0)->opcode() != BitAnd)
</ins><span class="cx">                     return false;
</span><del>-                if (right->opcode() != BitAnd || !canBeInternal(right))
-                    return false;
-                if (left->child(0)->opcode() != BitAnd || !canBeInternal(left->child(0)))
-                    return false;
</del><span class="cx"> 
</span><span class="cx">                 Value* nValue = left->child(0)->child(0);
</span><span class="cx">                 Value* maskValue1 = left->child(0)->child(1);
</span><span class="lines">@@ -2913,9 +2907,6 @@
</span><span class="cx">                 Tmp result = tmp(m_value);
</span><span class="cx">                 append(relaxedMoveForType(m_value->type()), tmp(dValue), result);
</span><span class="cx">                 append(opcode, tmp(nValue), imm(lsbValue), imm(width), result);
</span><del>-                commitInternal(left->child(0));
-                commitInternal(left);
-                commitInternal(right);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -2929,12 +2920,8 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(ExtractInsertBitfieldAtLowEnd32, ExtractInsertBitfieldAtLowEnd64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp)) 
</span><span class="cx">                     return false;
</span><del>-                if (left->opcode() != BitAnd || !canBeInternal(left))
</del><ins>+                if (left->opcode() != BitAnd || left->child(0)->opcode() != ZShr || right->opcode() != BitAnd)
</ins><span class="cx">                     return false;
</span><del>-                if (left->child(0)->opcode() != ZShr || !canBeInternal(left->child(0)))
-                    return false;
-                if (right->opcode() != BitAnd || !canBeInternal(right))
-                    return false;
</del><span class="cx"> 
</span><span class="cx">                 Value* nValue = left->child(0)->child(0);
</span><span class="cx">                 Value* maskValue1 = left->child(1);
</span><span class="lines">@@ -2969,9 +2956,6 @@
</span><span class="cx">                 Tmp result = tmp(m_value);
</span><span class="cx">                 append(relaxedMoveForType(m_value->type()), tmp(dValue), result);
</span><span class="cx">                 append(opcode, tmp(nValue), imm(lsbValue), imm(width), result);
</span><del>-                commitInternal(left->child(0));
-                commitInternal(left);
-                commitInternal(right);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -2983,7 +2967,7 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(OrNot32, OrNot64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Tmp)) 
</span><span class="cx">                     return false;
</span><del>-                if (right->opcode() != BitXor || !canBeInternal(right))
</del><ins>+                if (right->opcode() != BitXor)
</ins><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 Value* nValue = left;
</span><span class="lines">@@ -2993,7 +2977,6 @@
</span><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 append(opcode, tmp(nValue), tmp(mValue), tmp(m_value));
</span><del>-                commitInternal(right);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -3045,7 +3028,7 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(InsertUnsignedBitfieldInZero32, InsertUnsignedBitfieldInZero64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp))
</span><span class="cx">                     return false;
</span><del>-                if (left->opcode() != BitAnd || !canBeInternal(left))
</del><ins>+                if (left->opcode() != BitAnd)
</ins><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 Value* nValue = left->child(0);
</span><span class="lines">@@ -3063,7 +3046,6 @@
</span><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 append(opcode, tmp(nValue), imm(right), imm(width), tmp(m_value));
</span><del>-                commitInternal(left);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -3078,10 +3060,8 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(InsertSignedBitfieldInZero32, InsertSignedBitfieldInZero64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp))
</span><span class="cx">                     return false;
</span><del>-                if (left->opcode() != SShr || !canBeInternal(left))
</del><ins>+                if (left->opcode() != SShr || left->child(0)->opcode() != Shl)
</ins><span class="cx">                     return false;
</span><del>-                if (left->child(0)->opcode() != Shl || !canBeInternal(left->child(0)))
-                    return false;
</del><span class="cx"> 
</span><span class="cx">                 Value* srcValue = left->child(0)->child(0);
</span><span class="cx">                 Value* amount1Value = left->child(0)->child(1);
</span><span class="lines">@@ -3103,8 +3083,6 @@
</span><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 append(opcode, tmp(srcValue), imm(lsbValue), imm(width), tmp(m_value));
</span><del>-                commitInternal(left->child(0));
-                commitInternal(left);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span><span class="lines">@@ -3130,10 +3108,8 @@
</span><span class="cx">                 Air::Opcode opcode = opcodeForType(ExtractSignedBitfield32, ExtractSignedBitfield64, m_value->type());
</span><span class="cx">                 if (!isValidForm(opcode, Arg::Tmp, Arg::Imm, Arg::Imm, Arg::Tmp))
</span><span class="cx">                     return false;
</span><del>-                if (left->opcode() != Shl || !canBeInternal(left))
</del><ins>+                if (left->opcode() != Shl || (left->child(0)->opcode() != ZShr && left->child(0)->opcode() != SShr))
</ins><span class="cx">                     return false;
</span><del>-                if ((left->child(0)->opcode() != ZShr && left->child(0)->opcode() != SShr) || !canBeInternal(left->child(0)))
-                    return false;
</del><span class="cx"> 
</span><span class="cx">                 Value* srcValue = left->child(0)->child(0);
</span><span class="cx">                 Value* lsbValue = left->child(0)->child(1);
</span><span class="lines">@@ -3155,8 +3131,6 @@
</span><span class="cx">                     return false;
</span><span class="cx"> 
</span><span class="cx">                 append(opcode, tmp(srcValue), imm(lsbValue), imm(width), tmp(m_value));
</span><del>-                commitInternal(left->child(0));
-                commitInternal(left);
</del><span class="cx">                 return true;
</span><span class="cx">             };
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>