Title: [241783] trunk/Source/_javascript_Core
Revision
241783
Author
[email protected]
Date
2019-02-19 15:27:16 -0800 (Tue, 19 Feb 2019)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (241782 => 241783)


--- trunk/Source/_javascript_Core/ChangeLog	2019-02-19 23:18:27 UTC (rev 241782)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-02-19 23:27:16 UTC (rev 241783)
@@ -1,5 +1,22 @@
 2019-02-19  Robin Morisset  <[email protected]>
 
+        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-19  Robin Morisset  <[email protected]>
+
         CachedCall should not consider it UNLIKELY that it will not stack overflow
         https://bugs.webkit.org/show_bug.cgi?id=194831
 

Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (241782 => 241783)


--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp	2019-02-19 23:18:27 UTC (rev 241782)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp	2019-02-19 23:27:16 UTC (rev 241783)
@@ -1438,7 +1438,7 @@
         // 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;

Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (241782 => 241783)


--- trunk/Source/_javascript_Core/b3/testb3.cpp	2019-02-19 23:18:27 UTC (rev 241782)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp	2019-02-19 23:27:16 UTC (rev 241783)
@@ -3521,7 +3521,7 @@
     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 @@
         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 @@
     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 @@
     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));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to