Title: [229517] trunk/Source/_javascript_Core
Revision
229517
Author
utatane....@gmail.com
Date
2018-03-11 12:52:24 -0700 (Sun, 11 Mar 2018)

Log Message

[B3] Above/Below should be strength-reduced for comparison with 0
https://bugs.webkit.org/show_bug.cgi?id=183543

Reviewed by Filip Pizlo.

Above(0, x) and BelowEqual(0, x) can be converted to constants false and true respectively.
This can be seen in ArraySlice(0) case: `Select(Above(0, length), length, 0)` this should
be converted to `0`. This patch adds such a folding to comparisons.

We also fix B3ReduceStrength issue creating an orphan value. If a flipped value is folded to
a constant, we do not insert flipped value and make it an orphan. This issue causes JSC test
failure with this B3Const32/64Value change. With this patch, we create a flipped value only
when we fail to fold it to a constant.

* b3/B3Const32Value.cpp:
(JSC::B3::Const32Value::lessThanConstant const):
(JSC::B3::Const32Value::greaterThanConstant const):
(JSC::B3::Const32Value::lessEqualConstant const):
(JSC::B3::Const32Value::greaterEqualConstant const):
(JSC::B3::Const32Value::aboveConstant const):
(JSC::B3::Const32Value::belowConstant const):
(JSC::B3::Const32Value::aboveEqualConstant const):
(JSC::B3::Const32Value::belowEqualConstant const):
* b3/B3Const64Value.cpp:
(JSC::B3::Const64Value::lessThanConstant const):
(JSC::B3::Const64Value::greaterThanConstant const):
(JSC::B3::Const64Value::lessEqualConstant const):
(JSC::B3::Const64Value::greaterEqualConstant const):
(JSC::B3::Const64Value::aboveConstant const):
(JSC::B3::Const64Value::belowConstant const):
(JSC::B3::Const64Value::aboveEqualConstant const):
(JSC::B3::Const64Value::belowEqualConstant const):
* b3/B3ReduceStrength.cpp:
* b3/testb3.cpp:
(JSC::B3::int64Operands):
(JSC::B3::int32Operands):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (229516 => 229517)


--- trunk/Source/_javascript_Core/ChangeLog	2018-03-11 17:45:49 UTC (rev 229516)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-03-11 19:52:24 UTC (rev 229517)
@@ -1,3 +1,42 @@
+2018-03-11  Yusuke Suzuki  <utatane....@gmail.com>
+
+        [B3] Above/Below should be strength-reduced for comparison with 0
+        https://bugs.webkit.org/show_bug.cgi?id=183543
+
+        Reviewed by Filip Pizlo.
+
+        Above(0, x) and BelowEqual(0, x) can be converted to constants false and true respectively.
+        This can be seen in ArraySlice(0) case: `Select(Above(0, length), length, 0)` this should
+        be converted to `0`. This patch adds such a folding to comparisons.
+
+        We also fix B3ReduceStrength issue creating an orphan value. If a flipped value is folded to
+        a constant, we do not insert flipped value and make it an orphan. This issue causes JSC test
+        failure with this B3Const32/64Value change. With this patch, we create a flipped value only
+        when we fail to fold it to a constant.
+
+        * b3/B3Const32Value.cpp:
+        (JSC::B3::Const32Value::lessThanConstant const):
+        (JSC::B3::Const32Value::greaterThanConstant const):
+        (JSC::B3::Const32Value::lessEqualConstant const):
+        (JSC::B3::Const32Value::greaterEqualConstant const):
+        (JSC::B3::Const32Value::aboveConstant const):
+        (JSC::B3::Const32Value::belowConstant const):
+        (JSC::B3::Const32Value::aboveEqualConstant const):
+        (JSC::B3::Const32Value::belowEqualConstant const):
+        * b3/B3Const64Value.cpp:
+        (JSC::B3::Const64Value::lessThanConstant const):
+        (JSC::B3::Const64Value::greaterThanConstant const):
+        (JSC::B3::Const64Value::lessEqualConstant const):
+        (JSC::B3::Const64Value::greaterEqualConstant const):
+        (JSC::B3::Const64Value::aboveConstant const):
+        (JSC::B3::Const64Value::belowConstant const):
+        (JSC::B3::Const64Value::aboveEqualConstant const):
+        (JSC::B3::Const64Value::belowEqualConstant const):
+        * b3/B3ReduceStrength.cpp:
+        * b3/testb3.cpp:
+        (JSC::B3::int64Operands):
+        (JSC::B3::int32Operands):
+
 2018-03-10  Yusuke Suzuki  <utatane....@gmail.com>
 
         [FTL] Drop NewRegexp for String.prototype.match with RegExp + global flag

Modified: trunk/Source/_javascript_Core/b3/B3Const32Value.cpp (229516 => 229517)


--- trunk/Source/_javascript_Core/b3/B3Const32Value.cpp	2018-03-11 17:45:49 UTC (rev 229516)
+++ trunk/Source/_javascript_Core/b3/B3Const32Value.cpp	2018-03-11 19:52:24 UTC (rev 229517)
@@ -220,6 +220,9 @@
 
 TriState Const32Value::lessThanConstant(const Value* other) const
 {
+    // INT32_MAX < x is always false.
+    if (static_cast<int32_t>(m_value) == std::numeric_limits<int32_t>::max())
+        return FalseTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(m_value < other->asInt32());
@@ -227,6 +230,9 @@
 
 TriState Const32Value::greaterThanConstant(const Value* other) const
 {
+    // INT32_MIN > x is always false.
+    if (static_cast<int32_t>(m_value) == std::numeric_limits<int32_t>::min())
+        return FalseTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(m_value > other->asInt32());
@@ -234,6 +240,9 @@
 
 TriState Const32Value::lessEqualConstant(const Value* other) const
 {
+    // INT32_MIN <= x is always true.
+    if (static_cast<int32_t>(m_value) == std::numeric_limits<int32_t>::min())
+        return TrueTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(m_value <= other->asInt32());
@@ -241,6 +250,9 @@
 
 TriState Const32Value::greaterEqualConstant(const Value* other) const
 {
+    // INT32_MAX >= x is always true.
+    if (static_cast<int32_t>(m_value) == std::numeric_limits<int32_t>::max())
+        return TrueTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(m_value >= other->asInt32());
@@ -248,6 +260,9 @@
 
 TriState Const32Value::aboveConstant(const Value* other) const
 {
+    // UINT32_MIN > x is always false.
+    if (static_cast<uint32_t>(m_value) == std::numeric_limits<uint32_t>::min())
+        return FalseTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(static_cast<uint32_t>(m_value) > static_cast<uint32_t>(other->asInt32()));
@@ -255,6 +270,9 @@
 
 TriState Const32Value::belowConstant(const Value* other) const
 {
+    // UINT32_MAX < x is always false.
+    if (static_cast<uint32_t>(m_value) == std::numeric_limits<uint32_t>::max())
+        return FalseTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(static_cast<uint32_t>(m_value) < static_cast<uint32_t>(other->asInt32()));
@@ -262,6 +280,9 @@
 
 TriState Const32Value::aboveEqualConstant(const Value* other) const
 {
+    // UINT32_MAX >= x is always true.
+    if (static_cast<uint32_t>(m_value) == std::numeric_limits<uint32_t>::max())
+        return TrueTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(static_cast<uint32_t>(m_value) >= static_cast<uint32_t>(other->asInt32()));
@@ -269,6 +290,9 @@
 
 TriState Const32Value::belowEqualConstant(const Value* other) const
 {
+    // UINT32_MIN <= x is always true.
+    if (static_cast<uint32_t>(m_value) == std::numeric_limits<uint32_t>::min())
+        return TrueTriState;
     if (!other->hasInt32())
         return MixedTriState;
     return triState(static_cast<uint32_t>(m_value) <= static_cast<uint32_t>(other->asInt32()));

Modified: trunk/Source/_javascript_Core/b3/B3Const64Value.cpp (229516 => 229517)


--- trunk/Source/_javascript_Core/b3/B3Const64Value.cpp	2018-03-11 17:45:49 UTC (rev 229516)
+++ trunk/Source/_javascript_Core/b3/B3Const64Value.cpp	2018-03-11 19:52:24 UTC (rev 229517)
@@ -220,6 +220,9 @@
 
 TriState Const64Value::lessThanConstant(const Value* other) const
 {
+    // INT64_MAX < x is always false.
+    if (static_cast<int64_t>(m_value) == std::numeric_limits<int64_t>::max())
+        return FalseTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(m_value < other->asInt64());
@@ -227,6 +230,9 @@
 
 TriState Const64Value::greaterThanConstant(const Value* other) const
 {
+    // INT64_MIN > x is always false.
+    if (static_cast<int64_t>(m_value) == std::numeric_limits<int64_t>::min())
+        return FalseTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(m_value > other->asInt64());
@@ -234,6 +240,9 @@
 
 TriState Const64Value::lessEqualConstant(const Value* other) const
 {
+    // INT64_MIN <= x is always true.
+    if (static_cast<int64_t>(m_value) == std::numeric_limits<int64_t>::min())
+        return TrueTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(m_value <= other->asInt64());
@@ -241,6 +250,9 @@
 
 TriState Const64Value::greaterEqualConstant(const Value* other) const
 {
+    // INT64_MAX >= x is always true.
+    if (static_cast<int64_t>(m_value) == std::numeric_limits<int64_t>::max())
+        return TrueTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(m_value >= other->asInt64());
@@ -248,6 +260,9 @@
 
 TriState Const64Value::aboveConstant(const Value* other) const
 {
+    // UINT64_MIN > x is always false.
+    if (static_cast<uint64_t>(m_value) == std::numeric_limits<uint64_t>::min())
+        return FalseTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(static_cast<uint64_t>(m_value) > static_cast<uint64_t>(other->asInt64()));
@@ -255,6 +270,9 @@
 
 TriState Const64Value::belowConstant(const Value* other) const
 {
+    // UINT64_MAX < x is always false.
+    if (static_cast<uint64_t>(m_value) == std::numeric_limits<uint64_t>::max())
+        return FalseTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(static_cast<uint64_t>(m_value) < static_cast<uint64_t>(other->asInt64()));
@@ -262,6 +280,9 @@
 
 TriState Const64Value::aboveEqualConstant(const Value* other) const
 {
+    // UINT64_MAX >= x is always true.
+    if (static_cast<uint64_t>(m_value) == std::numeric_limits<uint64_t>::max())
+        return TrueTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(static_cast<uint64_t>(m_value) >= static_cast<uint64_t>(other->asInt64()));
@@ -269,6 +290,9 @@
 
 TriState Const64Value::belowEqualConstant(const Value* other) const
 {
+    // UINT64_MIN <= x is always true.
+    if (static_cast<uint64_t>(m_value) == std::numeric_limits<uint64_t>::min())
+        return TrueTriState;
     if (!other->hasInt64())
         return MixedTriState;
     return triState(static_cast<uint64_t>(m_value) <= static_cast<uint64_t>(other->asInt64()));

Modified: trunk/Source/_javascript_Core/b3/B3ReduceStrength.cpp (229516 => 229517)


--- trunk/Source/_javascript_Core/b3/B3ReduceStrength.cpp	2018-03-11 17:45:49 UTC (rev 229516)
+++ trunk/Source/_javascript_Core/b3/B3ReduceStrength.cpp	2018-03-11 19:52:24 UTC (rev 229517)
@@ -1673,32 +1673,32 @@
         case Below:
         case AboveEqual:
         case BelowEqual: {
-            auto* value = newComparisonVaueIfNecessary();
+            CanonicalizedComparison comparison = canonicalizeComparison(m_value);
             TriState result = MixedTriState;
-            switch (value->opcode()) {
+            switch (comparison.opcode) {
             case LessThan:
-                result = value->child(1)->greaterThanConstant(value->child(0));
+                result = comparison.operands[1]->greaterThanConstant(comparison.operands[0]);
                 break;
             case GreaterThan:
-                result = value->child(1)->lessThanConstant(value->child(0));
+                result = comparison.operands[1]->lessThanConstant(comparison.operands[0]);
                 break;
             case LessEqual:
-                result = value->child(1)->greaterEqualConstant(value->child(0));
+                result = comparison.operands[1]->greaterEqualConstant(comparison.operands[0]);
                 break;
             case GreaterEqual:
-                result = value->child(1)->lessEqualConstant(value->child(0));
+                result = comparison.operands[1]->lessEqualConstant(comparison.operands[0]);
                 break;
             case Above:
-                result = value->child(1)->belowConstant(value->child(0));
+                result = comparison.operands[1]->belowConstant(comparison.operands[0]);
                 break;
             case Below:
-                result = value->child(1)->aboveConstant(value->child(0));
+                result = comparison.operands[1]->aboveConstant(comparison.operands[0]);
                 break;
             case AboveEqual:
-                result = value->child(1)->belowEqualConstant(value->child(0));
+                result = comparison.operands[1]->belowEqualConstant(comparison.operands[0]);
                 break;
             case BelowEqual:
-                result = value->child(1)->aboveEqualConstant(value->child(0));
+                result = comparison.operands[1]->aboveEqualConstant(comparison.operands[0]);
                 break;
             default:
                 RELEASE_ASSERT_NOT_REACHED();
@@ -1705,14 +1705,14 @@
                 break;
             }
 
-            if (auto* constant = m_proc.addBoolConstant(value->origin(), result)) {
+            if (auto* constant = m_proc.addBoolConstant(m_value->origin(), result)) {
                 replaceWithNewValue(constant);
                 break;
             }
-
-            // Replace with newly created "value". Its opcode is flipped and operands are swapped from m_value.
-            if (m_value != value)
-                replaceWithNewValue(value);
+            if (comparison.opcode != m_value->opcode()) {
+                replaceWithNew<Value>(comparison.opcode, m_value->origin(), comparison.operands[0], comparison.operands[1]);
+                break;
+            }
             break;
         }
 
@@ -2166,7 +2166,11 @@
         }
     }
 
-    Value* newComparisonVaueIfNecessary()
+    struct CanonicalizedComparison {
+        Opcode opcode;
+        Value* operands[2];
+    };
+    static CanonicalizedComparison canonicalizeComparison(Value* value)
     {
         auto flip = [] (Opcode opcode) {
             switch (opcode) {
@@ -2190,11 +2194,9 @@
                 return opcode;
             }
         };
-        if (shouldSwapBinaryOperands(m_value)) {
-            m_changed = true;
-            return m_proc.add<Value>(flip(m_value->opcode()), m_value->origin(), m_value->child(1), m_value->child(0));
-        }
-        return m_value;
+        if (shouldSwapBinaryOperands(value))
+            return { flip(value->opcode()), { value->child(1), value->child(0) } };
+        return { value->opcode(), { value->child(0), value->child(1) } };
     }
 
     // FIXME: This should really be a forward analysis. Instead, we uses a bounded-search backwards

Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (229516 => 229517)


--- trunk/Source/_javascript_Core/b3/testb3.cpp	2018-03-11 17:45:49 UTC (rev 229516)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp	2018-03-11 19:52:24 UTC (rev 229517)
@@ -247,6 +247,10 @@
     operands.append({ "int64-min", std::numeric_limits<int64_t>::min() });
     operands.append({ "int32-max", std::numeric_limits<int32_t>::max() });
     operands.append({ "int32-min", std::numeric_limits<int32_t>::min() });
+    operands.append({ "uint64-max", static_cast<int64_t>(std::numeric_limits<uint64_t>::max()) });
+    operands.append({ "uint64-min", static_cast<int64_t>(std::numeric_limits<uint64_t>::min()) });
+    operands.append({ "uint32-max", static_cast<int64_t>(std::numeric_limits<uint32_t>::max()) });
+    operands.append({ "uint32-min", static_cast<int64_t>(std::numeric_limits<uint32_t>::min()) });
 
     return operands;
 }
@@ -260,7 +264,9 @@
         { "42", 42 },
         { "-42", -42 },
         { "int32-max", std::numeric_limits<int32_t>::max() },
-        { "int32-min", std::numeric_limits<int32_t>::min() }
+        { "int32-min", std::numeric_limits<int32_t>::min() },
+        { "uint32-max", static_cast<int32_t>(std::numeric_limits<uint32_t>::max()) },
+        { "uint32-min", static_cast<int32_t>(std::numeric_limits<uint32_t>::min()) }
     });
     return operands;
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to