Attachment 361341 Details for Bug 194250 – Patch

[patch] Patch

patch194250 (text/plain), 13.03 KB, created by Robin Morisset on 2019-02-06 16:08:02 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-02-06 16:08:02 PST

Size: 13.03 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 241105)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,38 @@
>+2019-02-06  Robin Morisset  <rmorisset@apple.com>
>+
>+        B3ReduceStrength: missing peephole optimizations for Neg and Sub
>+        https://bugs.webkit.org/show_bug.cgi?id=194250
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adds the following optimizations for integers:
>+        - Sub(x, x) => 0
>+            Already covered by the test testSubArg
>+        - Sub(x1, Neg(x2)) => Add (x1, x2)
>+            Added test: testSubNeg
>+        - Neg(Sub(x1, x2)) => Sub(x2, x1)
>+            Added test: testNegSub
>+        - Add(Neg(x1), x2) => Sub(x2, x1)
>+            Added test: testAddNeg1
>+        - Add(x1, Neg(x2)) => Sub(x1, x2)
>+            Added test: testAddNeg2
>+        Adds the following optimization for floating point values:
>+        - Abs(Neg(x)) => Abs(x)
>+            Added test: testAbsNegArg
>+            Adds the following optimization:
>+
>+        Also did some trivial refactoring, using m_value->isInteger() everywhere instead of isInt(m_value->type()), and using replaceWithNew<Value> instead of replaceWithNewValue(m_proc.add<Value(..))
>+
>+        * b3/B3ReduceStrength.cpp:
>+        * b3/testb3.cpp:
>+        (JSC::B3::testAddNeg1):
>+        (JSC::B3::testAddNeg2):
>+        (JSC::B3::testSubNeg):
>+        (JSC::B3::testNegSub):
>+        (JSC::B3::testAbsAbsArg):
>+        (JSC::B3::testAbsNegArg):
>+        (JSC::B3::run):
>+
> 2019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] PrivateName to PublicName hash table is wasteful
>Index: Source/JavaScriptCore/b3/B3ReduceStrength.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(revision 241105)
>+++ Source/JavaScriptCore/b3/B3ReduceStrength.cpp	(working copy)
>@@ -499,7 +499,7 @@ private:
>         case Add:
>             handleCommutativity();
>             
>-            if (m_value->child(0)->opcode() == Add && isInt(m_value->type())) {
>+            if (m_value->child(0)->opcode() == Add && m_value->isInteger()) {
>                 // Turn this: Add(Add(value, constant1), constant2)
>                 // Into this: Add(value, constant1 + constant2)
>                 Value* newSum = m_value->child(1)->addConstant(m_proc, m_value->child(0)->child(1));
>@@ -532,7 +532,7 @@ private:
>             
>             // Turn this: Add(otherValue, Add(value, constant))
>             // Into this: Add(Add(value, otherValue), constant)
>-            if (isInt(m_value->type())
>+            if (m_value->isInteger()
>                 && !m_value->child(0)->hasInt()
>                 && m_value->child(1)->opcode() == Add
>                 && m_value->child(1)->child(1)->hasInt()) {
>@@ -579,14 +579,29 @@ private:
>                 break;
>             }
> 
>-            // Turn this: Integer Add(Sub(0, value), -1)
>-            // Into this: BitXor(value, -1)
>-            if (m_value->isInteger()
>-                && m_value->child(0)->opcode() == Sub
>-                && m_value->child(1)->isInt(-1)
>-                && m_value->child(0)->child(0)->isInt(0)) {
>-                replaceWithNewValue(m_proc.add<Value>(BitXor, m_value->origin(), m_value->child(0)->child(1), m_value->child(1)));
>-                break;
>+            if (m_value->isInteger()) {
>+                // Turn this: Integer Add(value, Neg(otherValue))
>+                // Into this: Sub(value, otherValue)
>+                if (m_value->child(1)->opcode() == Neg) {
>+                    replaceWithNew<Value>(Sub, m_value->origin(), m_value->child(0), m_value->child(1)->child(0));
>+                    break;
>+                }
>+
>+                // Turn this: Integer Add(Neg(value), otherValue)
>+                // Into this: Sub(otherValue, value)
>+                if (m_value->child(0)->opcode() == Neg) {
>+                    replaceWithNew<Value>(Sub, m_value->origin(), m_value->child(1), m_value->child(0)->child(0));
>+                    break;
>+                }
>+
>+                // Turn this: Integer Add(Sub(0, value), -1)
>+                // Into this: BitXor(value, -1)
>+                if (m_value->child(0)->opcode() == Sub
>+                    && m_value->child(1)->isInt(-1)
>+                    && m_value->child(0)->child(0)->isInt(0)) {
>+                    replaceWithNew<Value>(BitXor, m_value->origin(), m_value->child(0)->child(1), m_value->child(1));
>+                    break;
>+                }
>             }
> 
>             break;
>@@ -599,7 +614,7 @@ private:
>                 break;
>             }
> 
>-            if (isInt(m_value->type())) {
>+            if (m_value->isInteger()) {
>                 // Turn this: Sub(value, constant)
>                 // Into this: Add(value, -constant)
>                 if (Value* negatedConstant = m_value->child(1)->negConstant(m_proc)) {
>@@ -615,6 +630,20 @@ private:
>                     replaceWithNew<Value>(Neg, m_value->origin(), m_value->child(1));
>                     break;
>                 }
>+
>+                // Turn this: Sub(value, value)
>+                // Into this: 0
>+                if (m_value->child(0) == m_value->child(1)) {
>+                    replaceWithNewValue(m_proc.addIntConstant(m_value, 0));
>+                    break;
>+                }
>+
>+                // Turn this: Sub(value, Neg(otherValue))
>+                // Into this: Add(value, otherValue)
>+                if (m_value->child(1)->opcode() == Neg) {
>+                    replaceWithNew<Value>(Add, m_value->origin(), m_value->child(0), m_value->child(1)->child(0));
>+                    break;
>+                }
>             }
> 
>             break;
>@@ -634,6 +663,13 @@ private:
>                 break;
>             }
>             
>+            // Turn this: Integer Neg(Sub(value, otherValue))
>+            // Into this: Sub(otherValue, value)
>+            if (m_value->isInteger() && m_value->child(0)->opcode() == Sub) {
>+                replaceWithNew<Value>(Sub, m_value->origin(), m_value->child(0)->child(1), m_value->child(0)->child(0));
>+                break;
>+            }
>+
>             break;
> 
>         case Mul:
>@@ -1201,6 +1237,13 @@ private:
>                 replaceWithIdentity(m_value->child(0));
>                 break;
>             }
>+                
>+            // Turn this: Abs(Neg(value))
>+            // Into this: Abs(value)
>+            if (m_value->child(0)->opcode() == Neg) {
>+                replaceWithIdentity(m_value->child(0)->child(0));
>+                break;
>+            }
> 
>             // Turn this: Abs(BitwiseCast(value))
>             // Into this: BitwiseCast(And(value, mask-top-bit))
>Index: Source/JavaScriptCore/b3/testb3.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/testb3.cpp	(revision 241105)
>+++ Source/JavaScriptCore/b3/testb3.cpp	(working copy)
>@@ -609,6 +609,36 @@ void testAddImmMem32(int32_t a, int32_t 
>     CHECK(inputOutput == a + b);
> }
> 
>+void testAddNeg1(int a, int b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    root->appendNewControlValue(
>+        proc, Return, Origin(),
>+        root->appendNew<Value>(
>+            proc, Add, Origin(),
>+            root->appendNew<Value>(proc, Neg, Origin(),
>+                root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0)),
>+            root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1)));
>+    
>+    CHECK(compileAndRun<int>(proc, a, b) == (- a) + b);
>+}
>+
>+void testAddNeg2(int a, int b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    root->appendNewControlValue(
>+        proc, Return, Origin(),
>+        root->appendNew<Value>(
>+            proc, Add, Origin(),
>+            root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0),
>+            root->appendNew<Value>(proc, Neg, Origin(),
>+                root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1))));
>+
>+    CHECK(compileAndRun<int>(proc, a, b) == a + (- b));
>+}
>+
> void testAddArgZeroImmZDef()
> {
>     Procedure proc;
>@@ -1897,6 +1927,36 @@ void testSubArgImm(int64_t a, int64_t b)
>     CHECK(compileAndRun<int64_t>(proc, a) == a - b);
> }
> 
>+void testSubNeg(int a, int b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    root->appendNewControlValue(
>+        proc, Return, Origin(),
>+        root->appendNew<Value>(
>+            proc, Sub, Origin(),
>+            root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0),
>+            root->appendNew<Value>(proc, Neg, Origin(),
>+                root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1))));
>+    
>+    CHECK(compileAndRun<int>(proc, a, b) == a - (- b));
>+}
>+
>+void testNegSub(int a, int b)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    root->appendNewControlValue(
>+        proc, Return, Origin(),
>+        root->appendNew<Value>(
>+            proc, Neg, Origin(),
>+            root->appendNew<Value>(proc, Sub, Origin(),
>+                root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0),
>+                root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1))));
>+
>+    CHECK(compileAndRun<int>(proc, a, b) == -(a - b));
>+}
>+
> void testNegValueSubOne(int a)
> {
>     Procedure proc;
>@@ -3917,7 +3977,19 @@ void testAbsAbsArg(double a)
>     Value* secondAbs = root->appendNew<Value>(proc, Abs, Origin(), firstAbs);
>     root->appendNewControlValue(proc, Return, Origin(), secondAbs);
> 
>-    CHECK(isIdentical(compileAndRun<double>(proc, a), fabs(a)));
>+    CHECK(isIdentical(compileAndRun<double>(proc, a), fabs(fabs(a))));
>+}
>+
>+void testAbsNegArg(double a)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    Value* neg = root->appendNew<Value>(proc, Abs, Origin(),
>+        root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0));
>+    Value* abs = root->appendNew<Value>(proc, Abs, Origin(), neg);
>+    root->appendNewControlValue(proc, Return, Origin(), abs);
>+    
>+    CHECK(isIdentical(compileAndRun<double>(proc, a), fabs(- a)));
> }
> 
> void testAbsBitwiseCastArg(double a)
>@@ -3997,7 +4069,21 @@ void testAbsAbsArg(float a)
>     Value* secondAbs = root->appendNew<Value>(proc, Abs, Origin(), firstAbs);
>     root->appendNewControlValue(proc, Return, Origin(), secondAbs);
> 
>-    CHECK(isIdentical(compileAndRun<float>(proc, bitwise_cast<int32_t>(a)), static_cast<float>(fabs(a))));
>+    CHECK(isIdentical(compileAndRun<float>(proc, bitwise_cast<int32_t>(a)), static_cast<float>(fabs(fabs(a)))));
>+}
>+
>+void testAbsNegArg(float a)
>+{
>+    Procedure proc;
>+    BasicBlock* root = proc.addBlock();
>+    Value* argument32 = root->appendNew<Value>(proc, Trunc, Origin(),
>+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
>+    Value* argument = root->appendNew<Value>(proc, BitwiseCast, Origin(), argument32);
>+    Value* neg = root->appendNew<Value>(proc, Neg, Origin(), argument);
>+    Value* abs = root->appendNew<Value>(proc, Abs, Origin(), neg);
>+    root->appendNewControlValue(proc, Return, Origin(), abs);
>+    
>+    CHECK(isIdentical(compileAndRun<float>(proc, bitwise_cast<int32_t>(a)), static_cast<float>(fabs(- a))));
> }
> 
> void testAbsBitwiseCastArg(float a)
>@@ -16382,6 +16468,8 @@ void run(const char* filter)
>     RUN_BINARY(testAddArgMem32, int32Operands(), int32Operands());
>     RUN_BINARY(testAddMemArg32, int32Operands(), int32Operands());
>     RUN_BINARY(testAddImmMem32, int32Operands(), int32Operands());
>+    RUN_BINARY(testAddNeg1, int32Operands(), int32Operands());
>+    RUN_BINARY(testAddNeg2, int32Operands(), int32Operands());
>     RUN(testAddArgZeroImmZDef());
>     RUN(testAddLoadTwice());
> 
>@@ -16549,6 +16637,8 @@ void run(const char* filter)
>     RUN_BINARY(testSubMemArg, int64Operands(), int64Operands());
>     RUN_BINARY(testSubImmMem, int32Operands(), int32Operands());
>     RUN_BINARY(testSubMemImm, int32Operands(), int32Operands());
>+    RUN_BINARY(testSubNeg, int32Operands(), int32Operands());
>+    RUN_BINARY(testNegSub, int32Operands(), int32Operands());
>     RUN_UNARY(testNegValueSubOne, int32Operands());
> 
>     RUN(testSubArgs32(1, 1));
>@@ -16905,12 +16995,14 @@ void run(const char* filter)
>     RUN_UNARY(testAbsImm, floatingPointOperands<double>());
>     RUN_UNARY(testAbsMem, floatingPointOperands<double>());
>     RUN_UNARY(testAbsAbsArg, floatingPointOperands<double>());
>+    RUN_UNARY(testAbsNegArg, floatingPointOperands<double>());
>     RUN_UNARY(testAbsBitwiseCastArg, floatingPointOperands<double>());
>     RUN_UNARY(testBitwiseCastAbsBitwiseCastArg, floatingPointOperands<double>());
>     RUN_UNARY(testAbsArg, floatingPointOperands<float>());
>     RUN_UNARY(testAbsImm, floatingPointOperands<float>());
>     RUN_UNARY(testAbsMem, floatingPointOperands<float>());
>     RUN_UNARY(testAbsAbsArg, floatingPointOperands<float>());
>+    RUN_UNARY(testAbsNegArg, floatingPointOperands<float>());
>     RUN_UNARY(testAbsBitwiseCastArg, floatingPointOperands<float>());
>     RUN_UNARY(testBitwiseCastAbsBitwiseCastArg, floatingPointOperands<float>());
>     RUN_UNARY(testAbsArgWithUselessDoubleConversion, floatingPointOperands<float>());

Actions: View | Formatted Diff | Diff

Attachments on bug 194250: 361134 | 361215 | 361341