Attachment 362436 Details for Bug 194625 – Patch

[patch] Patch

patch194625 (text/plain), 4.60 KB, created by Robin Morisset on 2019-02-19 14:49:54 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-02-19 14:49:54 PST

Size: 4.60 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 2037305202e..d63581f3834 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-02-19  Robin Morisset  <rmorisset@apple.com>
>+
>+        B3-O2 incorrectly optimizes this subtest
>+        https://bugs.webkit.org/show_bug.cgi?id=194625
>+
>+        Reviewed by Saam Barati.
>+
>+        Trivial fix. Instead of doing
>+            if (!cond) foo else bar => if (cond) bar else foo
>+        B3LowerToAir was doing
>+            if (x^C) foo else bar => if (cond) bar else foo whenever C&1, even if C was for example 3.
>+
>+        * b3/B3LowerToAir.cpp:
>+        * b3/testb3.cpp:
>+        (JSC::B3::testBitNotOnBooleanAndBranch32):
>+        (JSC::B3::testNotOnBooleanAndBranch32): Added.
>+
> 2019-02-18  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Introduce JSNonDestructibleProxy for JavaScriptCore.framework's GlobalThis
>diff --git a/Source/JavaScriptCore/b3/B3LowerToAir.cpp b/Source/JavaScriptCore/b3/B3LowerToAir.cpp
>index ba156bd25f9..1b3a92e5226 100644
>--- a/Source/JavaScriptCore/b3/B3LowerToAir.cpp
>+++ b/Source/JavaScriptCore/b3/B3LowerToAir.cpp
>@@ -1438,7 +1438,7 @@ private:
>         // we do need at least one iteration of it for Check.
>         for (;;) {
>             bool shouldInvert =
>-                (value->opcode() == BitXor && value->child(1)->hasInt() && (value->child(1)->asInt() & 1) && value->child(0)->returnsBool())
>+                (value->opcode() == BitXor && value->child(1)->hasInt() && (value->child(1)->asInt() == 1) && value->child(0)->returnsBool())
>                 || (value->opcode() == Equal && value->child(1)->isInt(0));
>             if (!shouldInvert)
>                 break;
>diff --git a/Source/JavaScriptCore/b3/testb3.cpp b/Source/JavaScriptCore/b3/testb3.cpp
>index b080e0dc50a..e0599e1bb61 100644
>--- a/Source/JavaScriptCore/b3/testb3.cpp
>+++ b/Source/JavaScriptCore/b3/testb3.cpp
>@@ -3521,7 +3521,7 @@ void testBitNotMem32(int32_t a)
>     CHECK(isIdentical(input, static_cast<int32_t>((static_cast<uint32_t>(a) ^ 0xffffffff))));
> }
> 
>-void testBitNotOnBooleanAndBranch32(int64_t a, int64_t b)
>+void testNotOnBooleanAndBranch32(int64_t a, int64_t b)
> {
>     Procedure proc;
>     BasicBlock* root = proc.addBlock();
>@@ -3534,7 +3534,7 @@ void testBitNotOnBooleanAndBranch32(int64_t a, int64_t b)
>         root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
>     Value* argsAreEqual = root->appendNew<Value>(proc, Equal, Origin(), arg1, arg2);
>     Value* argsAreNotEqual = root->appendNew<Value>(proc, BitXor, Origin(),
>-        root->appendNew<Const32Value>(proc, Origin(), -1),
>+        root->appendNew<Const32Value>(proc, Origin(), 1),
>         argsAreEqual);
> 
>     root->appendNewControlValue(
>@@ -3554,6 +3554,35 @@ void testBitNotOnBooleanAndBranch32(int64_t a, int64_t b)
>     CHECK(compileAndRun<int32_t>(proc, a, b) == expectedValue);
> }
> 
>+void testBitNotOnBooleanAndBranch32(int64_t a, int64_t b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    BasicBlock* thenCase = proc.addBlock();
>+    BasicBlock* elseCase = proc.addBlock();
>+
>+    Value* arg1 = root->appendNew<Value>(proc, Trunc, Origin(),
>+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
>+    Value* arg2 = root->appendNew<Value>(proc, Trunc, Origin(),
>+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
>+    Value* argsAreEqual = root->appendNew<Value>(proc, Equal, Origin(), arg1, arg2);
>+    Value* bitNotArgsAreEqual = root->appendNew<Value>(proc, BitXor, Origin(),
>+        root->appendNew<Const32Value>(proc, Origin(), -1),
>+        argsAreEqual);
>+
>+    root->appendNewControlValue(proc, Branch, Origin(),
>+        bitNotArgsAreEqual, FrequentedBlock(thenCase), FrequentedBlock(elseCase));
>+
>+    thenCase->appendNewControlValue(proc, Return, Origin(),
>+        thenCase->appendNew<Const32Value>(proc, Origin(), 42));
>+
>+    elseCase->appendNewControlValue(proc, Return, Origin(),
>+        elseCase->appendNew<Const32Value>(proc, Origin(), -42));
>+
>+    int32_t expectedValue = ~(a == b) ? 42 : -42; // always 42
>+    CHECK(compileAndRun<int32_t>(proc, a, b) == expectedValue);
>+}
>+
> void testShlArgs(int64_t a, int64_t b)
> {
>     Procedure proc;
>@@ -16858,6 +16887,7 @@ void run(const char* filter)
>     RUN_UNARY(testBitNotArg32, int32Operands());
>     RUN_UNARY(testBitNotImm32, int32Operands());
>     RUN_UNARY(testBitNotMem32, int32Operands());
>+    RUN_BINARY(testNotOnBooleanAndBranch32, int32Operands(), int32Operands());
>     RUN_BINARY(testBitNotOnBooleanAndBranch32, int32Operands(), int32Operands());
> 
>     RUN(testShlArgs(1, 0));

Actions: View | Formatted Diff | Diff

Attachments on bug 194625: 362428 | 362436