Attachment 364988 Details for Bug 152164 – Patch

[patch] Patch

patch152164 (text/plain), 6.55 KB, created by Robin Morisset on 2019-03-17 17:38:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-03-17 17:38:21 PDT

Size: 6.55 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 9687b92ca48..93a24e89b40 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-03-17  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-03-17  Diego Pino Garcia  <dpino@igalia.com>
> 
>         Fix WPE and GTK Debug builds after r243049
>diff --git a/Source/JavaScriptCore/b3/B3ReduceStrength.cpp b/Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>index 6701cbdb12b..0e3a65be240 100644
>--- a/Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>+++ b/Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>@@ -1200,6 +1200,18 @@ private:
>                 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;
> 
>diff --git a/Source/JavaScriptCore/b3/testb3.cpp b/Source/JavaScriptCore/b3/testb3.cpp
>index 2c75eb2acf5..0e371bc0181 100644
>--- a/Source/JavaScriptCore/b3/testb3.cpp
>+++ b/Source/JavaScriptCore/b3/testb3.cpp
>@@ -3777,6 +3777,7 @@ void testShlImms(int64_t a, int64_t b)
>             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));
> }
> 
>@@ -3791,9 +3792,28 @@ void testShlArgImm(int64_t a, int64_t b)
>             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;
>@@ -3837,6 +3857,7 @@ void testShlImms32(int32_t a, int32_t b)
>             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));
> }
> 
>@@ -3853,9 +3874,30 @@ void testShlArgImm32(int32_t a, int32_t b)
>                 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;
>@@ -17309,6 +17351,13 @@ void run(const char* filter)
>     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));
>@@ -17327,9 +17376,17 @@ void run(const char* filter)
>     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