Attachment 361733 Details for Bug 152164 – Patch

[patch] Patch

patch152164 (text/plain), 6.40 KB, created by Robin Morisset on 2019-02-11 16:58:31 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-02-11 16:58:31 PST

Size: 6.40 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 241283)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,26 @@
>+2019-02-11  Robin Morisset  <rmorisset@apple.com>
>+
>+        B3 should reduce Shl(<S|Z>Shr(@x, @const), @const) to BitAnd(@x, -(1<<@const))
>+        https://bugs.webkit.org/show_bug.cgi?id=152164
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Turn this: Shl(<S|Z>Shr(@x, @const), @const)
>+        Into this: BitAnd(@x, -(1<<@const))
>+
>+        I added two tests: one for ZShr/32 bits, and one for SShr/64 bits, I think it is enough coverage (no reason for any interaction between the signedness of the shift and the bitwidth).
>+        I also modified a few adjacent tests to remove undefined behaviours.
>+
>+        * b3/B3ReduceStrength.cpp:
>+        * b3/testb3.cpp:
>+        (JSC::B3::testShlImms):
>+        (JSC::B3::testShlArgImm):
>+        (JSC::B3::testShlSShrArgImm):
>+        (JSC::B3::testShlImms32):
>+        (JSC::B3::testShlArgImm32):
>+        (JSC::B3::testShlZShrArgImm32):
>+        (JSC::B3::run):
>+
> 2019-02-11  Mark Lam  <mark.lam@apple.com>
> 
>         Randomize insertion of deallocated StructureIDs into the StructureIDTable's free list.
>Index: Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(revision 241267)
>+++ Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(working copy)
>@@ -1127,6 +1127,18 @@
>                 break;
>             }
> 
>+            // Turn this: Shl(<S|Z>Shr(@x, @const), @const)
>+            // Into this: BitAnd(@x, -(1<<@const))
>+            if ((m_value->child(0)->opcode() == SShr || m_value->child(0)->opcode() == ZShr)
>+                && m_value->child(0)->child(1)->hasInt()
>+                && m_value->child(1)->hasInt()
>+                && m_value->child(0)->child(1)->asInt() == m_value->child(1)->asInt()) {
>+                int shiftAmount = m_value->child(1)->asInt() & (m_value->type() == Int32 ? 31 : 63);
>+                Value* newConst = m_proc.addIntConstant(m_value, - static_cast<int64_t>(1ull << shiftAmount));
>+                replaceWithNew<Value>(BitAnd, m_value->origin(), m_value->child(0)->child(0), newConst);
>+                break;
>+            }
>+
>             handleShiftAmount();
>             break;
> 
>Index: Source/JavaScriptCore/b3/testb3.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/testb3.cpp	(revision 241267)
>+++ Source/JavaScriptCore/b3/testb3.cpp	(working copy)
>@@ -3581,6 +3581,7 @@
>             root->appendNew<Const64Value>(proc, Origin(), a),
>             root->appendNew<Const32Value>(proc, Origin(), b)));
> 
>+    b = b & 0x3f; // to avoid undefined behaviour below
>     CHECK(compileAndRun<int64_t>(proc) == (a << b));
> }
> 
>@@ -3595,9 +3596,28 @@
>             root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0),
>             root->appendNew<Const32Value>(proc, Origin(), b)));
> 
>+    b = b & 0x3f; // to avoid undefined behaviour below
>     CHECK(compileAndRun<int64_t>(proc, a) == (a << b));
> }
> 
>+void testShlSShrArgImm(int64_t a, int64_t b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    Value* argA = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
>+    Value* constB = root->appendNew<Const32Value>(proc, Origin(), b);
>+    Value* innerShift = root->appendNew<Value>(proc, SShr, Origin(), argA, constB);
>+    root->appendNewControlValue(
>+        proc, Return, Origin(),
>+        root->appendNew<Value>(
>+            proc, Shl, Origin(),
>+            innerShift,
>+            constB));
>+
>+    b = b & 0x3f; // to avoid undefined behaviour below
>+    CHECK(compileAndRun<int64_t>(proc, a) == ((a >> b) << b));
>+}
>+
> void testShlArg32(int32_t a)
> {
>     Procedure proc;
>@@ -3641,6 +3661,7 @@
>             root->appendNew<Const32Value>(proc, Origin(), a),
>             root->appendNew<Const32Value>(proc, Origin(), b)));
> 
>+    b = b & 0x1f; // to avoid undefined behaviour below
>     CHECK(compileAndRun<int32_t>(proc) == (a << b));
> }
> 
>@@ -3657,9 +3678,30 @@
>                 root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0)),
>             root->appendNew<Const32Value>(proc, Origin(), b)));
> 
>+    b = b & 0x1f; // to avoid undefined behaviour below
>     CHECK(compileAndRun<int32_t>(proc, a) == (a << b));
> }
> 
>+void testShlZShrArgImm32(int32_t a, int32_t b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    Value* argA = root->appendNew<Value>(
>+        proc, Trunc, Origin(),
>+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
>+    Value* constB = root->appendNew<Const32Value>(proc, Origin(), b);
>+    Value* innerShift = root->appendNew<Value>(proc, ZShr, Origin(), argA, constB);
>+    root->appendNewControlValue(
>+        proc, Return, Origin(),
>+        root->appendNew<Value>(
>+            proc, Shl, Origin(),
>+            innerShift,
>+            constB));
>+
>+    b = b & 0x1f; // to avoid undefined behaviour below
>+    CHECK(compileAndRun<int32_t>(proc, a) == static_cast<int32_t>((static_cast<uint32_t>(a) >> b) << b));
>+}
>+
> void testSShrArgs(int64_t a, int64_t b)
> {
>     Procedure proc;
>@@ -16880,6 +16922,13 @@
>     RUN(testShlArgImm(0xffffffffffffffff, 0));
>     RUN(testShlArgImm(0xffffffffffffffff, 1));
>     RUN(testShlArgImm(0xffffffffffffffff, 63));
>+    RUN(testShlSShrArgImm(1, 0));
>+    RUN(testShlSShrArgImm(1, 1));
>+    RUN(testShlSShrArgImm(1, 62));
>+    RUN(testShlSShrArgImm(1, 65));
>+    RUN(testShlSShrArgImm(0xffffffffffffffff, 0));
>+    RUN(testShlSShrArgImm(0xffffffffffffffff, 1));
>+    RUN(testShlSShrArgImm(0xffffffffffffffff, 63));
>     RUN(testShlArg32(2));
>     RUN(testShlArgs32(1, 0));
>     RUN(testShlArgs32(1, 1));
>@@ -16898,9 +16947,17 @@
>     RUN(testShlArgImm32(1, 0));
>     RUN(testShlArgImm32(1, 1));
>     RUN(testShlArgImm32(1, 62));
>+    RUN(testShlArgImm32(1, 33));
>     RUN(testShlArgImm32(0xffffffff, 0));
>     RUN(testShlArgImm32(0xffffffff, 1));
>     RUN(testShlArgImm32(0xffffffff, 63));
>+    RUN(testShlZShrArgImm32(1, 0));
>+    RUN(testShlZShrArgImm32(1, 1));
>+    RUN(testShlZShrArgImm32(1, 62));
>+    RUN(testShlZShrArgImm32(1, 33));
>+    RUN(testShlZShrArgImm32(0xffffffff, 0));
>+    RUN(testShlZShrArgImm32(0xffffffff, 1));
>+    RUN(testShlZShrArgImm32(0xffffffff, 63));
> 
>     RUN(testSShrArgs(1, 0));
>     RUN(testSShrArgs(1, 1));

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 152164: 361732 | 361733 | 361754 | 361755 | 361833 | 361852 | 364988 | 364997 | 365002