[JSC] Support Doubles with B3's Add
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Nov 2015 05:12:20 +0000 (05:12 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Nov 2015 05:12:20 +0000 (05:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151164

Patch by Benjamin Poulain <bpoulain@apple.com> on 2015-11-11
Reviewed by Filip Pizlo.

I tweaked ReduceStrength a bit to maintain correctness.
Nothing fancy otherwise.

* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::lower):
* b3/B3ReduceStrength.cpp:
* b3/B3Value.h:
* b3/B3ValueInlines.h:
(JSC::B3::Value::isInteger):
* b3/air/AirOpcode.opcodes:
* b3/testb3.cpp:
(JSC::B3::bitWiseEqual):
(JSC::B3::testAddArgDouble):
(JSC::B3::testAddArgsDouble):
(JSC::B3::testAddArgImmDouble):
(JSC::B3::testAddImmArgDouble):
(JSC::B3::testAddImmsDouble):
(JSC::B3::run):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@192347 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3LowerToAir.cpp
Source/JavaScriptCore/b3/B3ReduceStrength.cpp
Source/JavaScriptCore/b3/B3Value.h
Source/JavaScriptCore/b3/B3ValueInlines.h
Source/JavaScriptCore/b3/air/AirOpcode.opcodes
Source/JavaScriptCore/b3/testb3.cpp

index 6b99208..00f5711 100644 (file)
@@ -1,3 +1,29 @@
+2015-11-11  Benjamin Poulain  <bpoulain@apple.com>
+
+        [JSC] Support Doubles with B3's Add
+        https://bugs.webkit.org/show_bug.cgi?id=151164
+
+        Reviewed by Filip Pizlo.
+
+        I tweaked ReduceStrength a bit to maintain correctness.
+        Nothing fancy otherwise.
+
+        * b3/B3LowerToAir.cpp:
+        (JSC::B3::Air::LowerToAir::lower):
+        * b3/B3ReduceStrength.cpp:
+        * b3/B3Value.h:
+        * b3/B3ValueInlines.h:
+        (JSC::B3::Value::isInteger):
+        * b3/air/AirOpcode.opcodes:
+        * b3/testb3.cpp:
+        (JSC::B3::bitWiseEqual):
+        (JSC::B3::testAddArgDouble):
+        (JSC::B3::testAddArgsDouble):
+        (JSC::B3::testAddArgImmDouble):
+        (JSC::B3::testAddImmArgDouble):
+        (JSC::B3::testAddImmsDouble):
+        (JSC::B3::run):
+
 2015-11-11  Filip Pizlo  <fpizlo@apple.com>
 
         B3 should be able to compile a program with Switch
index 5bfbed4..e02b028 100644 (file)
@@ -1218,9 +1218,7 @@ private:
         }
 
         case Add: {
-            // FIXME: Need a story for doubles.
-            // https://bugs.webkit.org/show_bug.cgi?id=150991
-            appendBinOp<Add32, Add64, Air::Oops, Commutative>(
+            appendBinOp<Add32, Add64, AddDouble, Commutative>(
                 m_value->child(0), m_value->child(1));
             return;
         }
index e295d5e..9ae0e7c 100644 (file)
@@ -156,10 +156,10 @@ private:
                 break;
             }
 
-            // Turn this: Add(value, value)
+            // Turn this: Integer Add(value, value)
             // Into this: Shl(value, 1)
             // This is a useful canonicalization. It's not meant to be a strength reduction.
-            if (m_value->child(0) == m_value->child(1)) {
+            if (m_value->isInteger() && m_value->child(0) == m_value->child(1)) {
                 replaceWithNewValue(
                     m_proc.add<Value>(
                         Shl, m_value->origin(), m_value->child(0),
@@ -169,7 +169,13 @@ private:
 
             // Turn this: Add(value, zero)
             // Into an Identity.
-            if (m_value->child(1)->isInt(0)) {
+            //
+            // Addition is subtle with doubles. Zero is not the neutral value, negative zero is:
+            //    0 + 0 = 0
+            //    0 + -0 = 0
+            //    -0 + 0 = 0
+            //    -0 + -0 = -0
+            if (m_value->child(1)->isInt(0) || m_value->child(1)->isNegativeZero()) {
                 m_value->replaceWithIdentity(m_value->child(0));
                 m_changed = true;
                 break;
index 5949cbf..bbe819a 100644 (file)
@@ -108,6 +108,7 @@ public:
     // ourselves to any particular idiom.
 
     bool isConstant() const;
+    bool isInteger() const;
     
     virtual Value* negConstant(Procedure&) const;
     virtual Value* addConstant(Procedure&, int32_t other) const;
@@ -167,6 +168,8 @@ public:
     // possible return values are 0 or 1. It's OK for this method to conservatively return false.
     bool returnsBool() const;
 
+    bool isNegativeZero() const;
+
     TriState asTriState() const;
     bool isLikeZero() const { return asTriState() == FalseTriState; }
     bool isLikeNonZero() const { return asTriState() == TrueTriState; }
index 543f162..34414f2 100644 (file)
@@ -64,6 +64,11 @@ inline bool Value::isConstant() const
     }
 }
 
+inline bool Value::isInteger() const
+{
+    return type() == Int32 || type() == Int64;
+}
+
 inline bool Value::hasInt32() const
 {
     return !!as<Const32Value>();
@@ -148,6 +153,15 @@ inline bool Value::hasNumber() const
     return hasInt() || hasDouble();
 }
 
+inline bool Value::isNegativeZero() const
+{
+    if (hasDouble()) {
+        double value = asDouble();
+        return !value && std::signbit(value);
+    }
+    return false;
+}
+
 template<typename T>
 inline bool Value::representableAs() const
 {
index 2a794d8..266d319 100644 (file)
@@ -132,6 +132,10 @@ And64 U:G, UD:G
     Tmp, Tmp
     Imm, Tmp
 
+AddDouble U:F, UD:F
+    Tmp, Tmp
+    Addr, Tmp
+
 Lshift32 U:G, UD:G
     Tmp*, Tmp
     Imm, Tmp
index b91ede5..c598021 100644 (file)
@@ -43,6 +43,7 @@
 #include "JSCInlines.h"
 #include "LinkBuffer.h"
 #include "VM.h"
+#include <cmath>
 #include <wtf/Lock.h>
 #include <wtf/NumberOfCores.h>
 #include <wtf/Threading.h>
@@ -226,6 +227,70 @@ void testAddLoadTwice()
     test(1);
 }
 
+void testAddArgDouble(double a)
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* value = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, Add, Origin(), value, value));
+
+    CHECK(isIdentical(compileAndRun<double>(proc, a), a + a));
+}
+
+void testAddArgsDouble(double a, double b)
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* valueA = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0);
+    Value* valueB = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR1);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, Add, Origin(), valueA, valueB));
+
+    CHECK(isIdentical(compileAndRun<double>(proc, a, b), a + b));
+}
+
+void testAddArgImmDouble(double a, double b)
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* valueA = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0);
+    Value* valueB = root->appendNew<ConstDoubleValue>(proc, Origin(), b);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, Add, Origin(), valueA, valueB));
+
+    CHECK(isIdentical(compileAndRun<double>(proc, a), a + b));
+}
+
+void testAddImmArgDouble(double a, double b)
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* valueA = root->appendNew<ConstDoubleValue>(proc, Origin(), a);
+    Value* valueB = root->appendNew<ArgumentRegValue>(proc, Origin(), FPRInfo::argumentFPR0);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, Add, Origin(), valueA, valueB));
+
+    CHECK(isIdentical(compileAndRun<double>(proc, b), a + b));
+}
+
+void testAddImmsDouble(double a, double b)
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* valueA = root->appendNew<ConstDoubleValue>(proc, Origin(), a);
+    Value* valueB = root->appendNew<ConstDoubleValue>(proc, Origin(), b);
+    root->appendNew<ControlValue>(
+        proc, Return, Origin(),
+        root->appendNew<Value>(proc, Add, Origin(), valueA, valueB));
+
+    CHECK(isIdentical(compileAndRun<double>(proc), a + b));
+}
+
 void testMulArg(int a)
 {
     Procedure proc;
@@ -3272,6 +3337,17 @@ void testSwitchChillDiv(unsigned degree, unsigned gap = 1)
     CHECK(!invoke<int32_t>(*code, degree * gap + 1, 42, 11));
 }
 
+// Make sure the compiler does not try to optimize anything out.
+NEVER_INLINE double zero()
+{
+    return 0.;
+}
+
+double negativeZero()
+{
+    return -zero();
+}
+
 #define RUN(test) do {                          \
         if (!shouldRun(#test))                  \
             break;                              \
@@ -3314,6 +3390,30 @@ void run(const char* filter)
     RUN(testAddArgs32(1, 2));
     RUN(testAddLoadTwice());
 
+    RUN(testAddArgDouble(M_PI));
+    RUN(testAddArgsDouble(M_PI, 1));
+    RUN(testAddArgsDouble(M_PI, -M_PI));
+    RUN(testAddArgImmDouble(M_PI, 1));
+    RUN(testAddArgImmDouble(M_PI, 0));
+    RUN(testAddArgImmDouble(M_PI, negativeZero()));
+    RUN(testAddArgImmDouble(0, 0));
+    RUN(testAddArgImmDouble(0, negativeZero()));
+    RUN(testAddArgImmDouble(negativeZero(), 0));
+    RUN(testAddArgImmDouble(negativeZero(), negativeZero()));
+    RUN(testAddImmArgDouble(M_PI, 1));
+    RUN(testAddImmArgDouble(M_PI, 0));
+    RUN(testAddImmArgDouble(M_PI, negativeZero()));
+    RUN(testAddImmArgDouble(0, 0));
+    RUN(testAddImmArgDouble(0, negativeZero()));
+    RUN(testAddImmArgDouble(negativeZero(), 0));
+    RUN(testAddImmArgDouble(negativeZero(), negativeZero()));
+    RUN(testAddImmsDouble(M_PI, 1));
+    RUN(testAddImmsDouble(M_PI, 0));
+    RUN(testAddImmsDouble(M_PI, negativeZero()));
+    RUN(testAddImmsDouble(0, 0));
+    RUN(testAddImmsDouble(0, negativeZero()));
+    RUN(testAddImmsDouble(negativeZero(), negativeZero()));
+
     RUN(testMulArg(5));
     RUN(testMulArgs(1, 1));
     RUN(testMulArgs(1, 2));
@@ -3787,7 +3887,7 @@ void run(const char* filter)
     RUN(testCallFunctionWithHellaArguments());
 
     RUN(testReturnDouble(0.0));
-    RUN(testReturnDouble(-0.0));
+    RUN(testReturnDouble(negativeZero()));
     RUN(testReturnDouble(42.5));
 
     RUN(testCallSimpleDouble(1, 2));