FTL B3 does not logicalNot correctly
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Dec 2015 21:59:59 +0000 (21:59 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Dec 2015 21:59:59 +0000 (21:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152512

Reviewed by Saam Barati.

Source/JavaScriptCore:

I'm working on a bug where V8/richards does not run correctly. I noticed that the codegen was
doing a log of Not32's followed by branches, which smelled like badness. To debug this, I
needed B3's origins to dump as something other than a hexed pointer to a node. The node index
would be better. So, I added the notion of an origin printer to Procedure.

The bug was easy enough to fix. This introduces Output::logicalNot(). In LLVM, it's the same
as bitNot(). In B3, it's compiled to Equal(value, 0). We could have also compiled it to
BitXor(value, 1), except that B3 will strength-reduce to that anyway whenever it's safe. It's
sort of nice that right now, you could use logicalNot() on non-bool values and get C-like
behavior.

Richards still doesn't run, though. There are more bugs!

* JavaScriptCore.xcodeproj/project.pbxproj:
* b3/B3BasicBlock.cpp:
(JSC::B3::BasicBlock::dump):
(JSC::B3::BasicBlock::deepDump):
* b3/B3BasicBlock.h:
(JSC::B3::BasicBlock::frequency):
(JSC::B3::DeepBasicBlockDump::DeepBasicBlockDump):
(JSC::B3::DeepBasicBlockDump::dump):
(JSC::B3::deepDump):
* b3/B3LowerToAir.cpp:
(JSC::B3::Air::LowerToAir::run):
(JSC::B3::Air::LowerToAir::lower):
* b3/B3Origin.h:
(JSC::B3::Origin::data):
* b3/B3OriginDump.h: Added.
(JSC::B3::OriginDump::OriginDump):
(JSC::B3::OriginDump::dump):
* b3/B3Procedure.cpp:
(JSC::B3::Procedure::~Procedure):
(JSC::B3::Procedure::printOrigin):
(JSC::B3::Procedure::addBlock):
(JSC::B3::Procedure::dump):
* b3/B3Procedure.h:
(JSC::B3::Procedure::setOriginPrinter):
* b3/B3Value.cpp:
(JSC::B3::Value::dumpChildren):
(JSC::B3::Value::deepDump):
* b3/B3Value.h:
(JSC::B3::DeepValueDump::DeepValueDump):
(JSC::B3::DeepValueDump::dump):
(JSC::B3::deepDump):
* ftl/FTLB3Output.cpp:
(JSC::FTL::Output::lockedStackSlot):
(JSC::FTL::Output::bitNot):
(JSC::FTL::Output::logicalNot):
(JSC::FTL::Output::load):
* ftl/FTLB3Output.h:
(JSC::FTL::Output::aShr):
(JSC::FTL::Output::lShr):
(JSC::FTL::Output::ctlz32):
(JSC::FTL::Output::addWithOverflow32):
(JSC::FTL::Output::lessThanOrEqual):
(JSC::FTL::Output::doubleEqual):
(JSC::FTL::Output::doubleEqualOrUnordered):
(JSC::FTL::Output::doubleNotEqualOrUnordered):
(JSC::FTL::Output::doubleLessThan):
(JSC::FTL::Output::doubleLessThanOrEqual):
(JSC::FTL::Output::doubleGreaterThan):
(JSC::FTL::Output::doubleGreaterThanOrEqual):
(JSC::FTL::Output::doubleNotEqualAndOrdered):
(JSC::FTL::Output::doubleLessThanOrUnordered):
(JSC::FTL::Output::doubleLessThanOrEqualOrUnordered):
(JSC::FTL::Output::doubleGreaterThanOrUnordered):
(JSC::FTL::Output::doubleGreaterThanOrEqualOrUnordered):
(JSC::FTL::Output::isZero32):
(JSC::FTL::Output::notZero32):
(JSC::FTL::Output::addIncomingToPhi):
(JSC::FTL::Output::bitCast):
(JSC::FTL::Output::bitNot): Deleted.
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileCheckArray):
(JSC::FTL::DFG::LowerDFGToLLVM::compileGetTypedArrayByteOffset):
(JSC::FTL::DFG::LowerDFGToLLVM::compileLogicalNot):
(JSC::FTL::DFG::LowerDFGToLLVM::compileCallOrConstruct):
(JSC::FTL::DFG::LowerDFGToLLVM::compileInstanceOfCustom):
(JSC::FTL::DFG::LowerDFGToLLVM::compileCountExecution):
(JSC::FTL::DFG::LowerDFGToLLVM::boolify):
(JSC::FTL::DFG::LowerDFGToLLVM::isMisc):
(JSC::FTL::DFG::LowerDFGToLLVM::isNotBoolean):
(JSC::FTL::DFG::LowerDFGToLLVM::isBoolean):
(JSC::FTL::DFG::LowerDFGToLLVM::unboxBoolean):
(JSC::FTL::DFG::LowerDFGToLLVM::isNotType):
(JSC::FTL::DFG::LowerDFGToLLVM::speculateObject):
* ftl/FTLOutput.h:
(JSC::FTL::Output::aShr):
(JSC::FTL::Output::lShr):
(JSC::FTL::Output::bitNot):
(JSC::FTL::Output::logicalNot):
(JSC::FTL::Output::insertElement):
* ftl/FTLState.cpp:
(JSC::FTL::State::State):

Source/WTF:

This change introduces yet another use of SharedTask in JSC. While doing this, I noticed that
SharedTask::run() always demands that whatever arguments the callback takes, they must be
passed as rvalue references. This was a clear misuse of perfect forwarding. This change makes
SharedTask's approach to forwarding match what we were already doing in ScopedLambda.

* wtf/SharedTask.h:

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

18 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/b3/B3BasicBlock.cpp
Source/JavaScriptCore/b3/B3BasicBlock.h
Source/JavaScriptCore/b3/B3LowerToAir.cpp
Source/JavaScriptCore/b3/B3Origin.h
Source/JavaScriptCore/b3/B3OriginDump.h [new file with mode: 0644]
Source/JavaScriptCore/b3/B3Procedure.cpp
Source/JavaScriptCore/b3/B3Procedure.h
Source/JavaScriptCore/b3/B3Value.cpp
Source/JavaScriptCore/b3/B3Value.h
Source/JavaScriptCore/ftl/FTLB3Output.cpp
Source/JavaScriptCore/ftl/FTLB3Output.h
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
Source/JavaScriptCore/ftl/FTLOutput.h
Source/JavaScriptCore/ftl/FTLState.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/SharedTask.h

index 58af33e..f818b0d 100644 (file)
@@ -1,3 +1,105 @@
+2015-12-22  Filip Pizlo  <fpizlo@apple.com>
+
+        FTL B3 does not logicalNot correctly
+        https://bugs.webkit.org/show_bug.cgi?id=152512
+
+        Reviewed by Saam Barati.
+
+        I'm working on a bug where V8/richards does not run correctly. I noticed that the codegen was
+        doing a log of Not32's followed by branches, which smelled like badness. To debug this, I
+        needed B3's origins to dump as something other than a hexed pointer to a node. The node index
+        would be better. So, I added the notion of an origin printer to Procedure.
+
+        The bug was easy enough to fix. This introduces Output::logicalNot(). In LLVM, it's the same
+        as bitNot(). In B3, it's compiled to Equal(value, 0). We could have also compiled it to
+        BitXor(value, 1), except that B3 will strength-reduce to that anyway whenever it's safe. It's
+        sort of nice that right now, you could use logicalNot() on non-bool values and get C-like
+        behavior.
+
+        Richards still doesn't run, though. There are more bugs!
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * b3/B3BasicBlock.cpp:
+        (JSC::B3::BasicBlock::dump):
+        (JSC::B3::BasicBlock::deepDump):
+        * b3/B3BasicBlock.h:
+        (JSC::B3::BasicBlock::frequency):
+        (JSC::B3::DeepBasicBlockDump::DeepBasicBlockDump):
+        (JSC::B3::DeepBasicBlockDump::dump):
+        (JSC::B3::deepDump):
+        * b3/B3LowerToAir.cpp:
+        (JSC::B3::Air::LowerToAir::run):
+        (JSC::B3::Air::LowerToAir::lower):
+        * b3/B3Origin.h:
+        (JSC::B3::Origin::data):
+        * b3/B3OriginDump.h: Added.
+        (JSC::B3::OriginDump::OriginDump):
+        (JSC::B3::OriginDump::dump):
+        * b3/B3Procedure.cpp:
+        (JSC::B3::Procedure::~Procedure):
+        (JSC::B3::Procedure::printOrigin):
+        (JSC::B3::Procedure::addBlock):
+        (JSC::B3::Procedure::dump):
+        * b3/B3Procedure.h:
+        (JSC::B3::Procedure::setOriginPrinter):
+        * b3/B3Value.cpp:
+        (JSC::B3::Value::dumpChildren):
+        (JSC::B3::Value::deepDump):
+        * b3/B3Value.h:
+        (JSC::B3::DeepValueDump::DeepValueDump):
+        (JSC::B3::DeepValueDump::dump):
+        (JSC::B3::deepDump):
+        * ftl/FTLB3Output.cpp:
+        (JSC::FTL::Output::lockedStackSlot):
+        (JSC::FTL::Output::bitNot):
+        (JSC::FTL::Output::logicalNot):
+        (JSC::FTL::Output::load):
+        * ftl/FTLB3Output.h:
+        (JSC::FTL::Output::aShr):
+        (JSC::FTL::Output::lShr):
+        (JSC::FTL::Output::ctlz32):
+        (JSC::FTL::Output::addWithOverflow32):
+        (JSC::FTL::Output::lessThanOrEqual):
+        (JSC::FTL::Output::doubleEqual):
+        (JSC::FTL::Output::doubleEqualOrUnordered):
+        (JSC::FTL::Output::doubleNotEqualOrUnordered):
+        (JSC::FTL::Output::doubleLessThan):
+        (JSC::FTL::Output::doubleLessThanOrEqual):
+        (JSC::FTL::Output::doubleGreaterThan):
+        (JSC::FTL::Output::doubleGreaterThanOrEqual):
+        (JSC::FTL::Output::doubleNotEqualAndOrdered):
+        (JSC::FTL::Output::doubleLessThanOrUnordered):
+        (JSC::FTL::Output::doubleLessThanOrEqualOrUnordered):
+        (JSC::FTL::Output::doubleGreaterThanOrUnordered):
+        (JSC::FTL::Output::doubleGreaterThanOrEqualOrUnordered):
+        (JSC::FTL::Output::isZero32):
+        (JSC::FTL::Output::notZero32):
+        (JSC::FTL::Output::addIncomingToPhi):
+        (JSC::FTL::Output::bitCast):
+        (JSC::FTL::Output::bitNot): Deleted.
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileCheckArray):
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileGetTypedArrayByteOffset):
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileLogicalNot):
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileCallOrConstruct):
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileInstanceOfCustom):
+        (JSC::FTL::DFG::LowerDFGToLLVM::compileCountExecution):
+        (JSC::FTL::DFG::LowerDFGToLLVM::boolify):
+        (JSC::FTL::DFG::LowerDFGToLLVM::isMisc):
+        (JSC::FTL::DFG::LowerDFGToLLVM::isNotBoolean):
+        (JSC::FTL::DFG::LowerDFGToLLVM::isBoolean):
+        (JSC::FTL::DFG::LowerDFGToLLVM::unboxBoolean):
+        (JSC::FTL::DFG::LowerDFGToLLVM::isNotType):
+        (JSC::FTL::DFG::LowerDFGToLLVM::speculateObject):
+        * ftl/FTLOutput.h:
+        (JSC::FTL::Output::aShr):
+        (JSC::FTL::Output::lShr):
+        (JSC::FTL::Output::bitNot):
+        (JSC::FTL::Output::logicalNot):
+        (JSC::FTL::Output::insertElement):
+        * ftl/FTLState.cpp:
+        (JSC::FTL::State::State):
+
 2015-12-22  Keith Miller  <keith_miller@apple.com>
 
         Remove OverridesHasInstance from TypeInfoFlags
index 8cdf5c2..7af1f65 100644 (file)
                0F48532A187DFDEC0083B687 /* FTLRecoveryOpcode.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F485326187DFDEC0083B687 /* FTLRecoveryOpcode.h */; settings = {ATTRIBUTES = (Private, ); }; };
                0F493AFA16D0CAD30084508B /* SourceProvider.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F493AF816D0CAD10084508B /* SourceProvider.cpp */; };
                0F4B94DC17B9F07500DD03A4 /* TypedArrayInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F4B94DB17B9F07500DD03A4 /* TypedArrayInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
+               0F4C91661C29F4F2004341A6 /* B3OriginDump.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F4C91651C29F4F2004341A6 /* B3OriginDump.h */; };
                0F4F29DF18B6AD1C0057BC15 /* DFGStaticExecutionCountEstimationPhase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F4F29DD18B6AD1C0057BC15 /* DFGStaticExecutionCountEstimationPhase.cpp */; };
                0F4F29E018B6AD1C0057BC15 /* DFGStaticExecutionCountEstimationPhase.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F4F29DE18B6AD1C0057BC15 /* DFGStaticExecutionCountEstimationPhase.h */; };
                0F50AF3C193E8B3900674EE8 /* DFGStructureClobberState.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F50AF3B193E8B3900674EE8 /* DFGStructureClobberState.h */; };
                0F485326187DFDEC0083B687 /* FTLRecoveryOpcode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = FTLRecoveryOpcode.h; path = ftl/FTLRecoveryOpcode.h; sourceTree = "<group>"; };
                0F493AF816D0CAD10084508B /* SourceProvider.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SourceProvider.cpp; sourceTree = "<group>"; };
                0F4B94DB17B9F07500DD03A4 /* TypedArrayInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TypedArrayInlines.h; sourceTree = "<group>"; };
+               0F4C91651C29F4F2004341A6 /* B3OriginDump.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = B3OriginDump.h; path = b3/B3OriginDump.h; sourceTree = "<group>"; };
                0F4F29DD18B6AD1C0057BC15 /* DFGStaticExecutionCountEstimationPhase.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGStaticExecutionCountEstimationPhase.cpp; path = dfg/DFGStaticExecutionCountEstimationPhase.cpp; sourceTree = "<group>"; };
                0F4F29DE18B6AD1C0057BC15 /* DFGStaticExecutionCountEstimationPhase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGStaticExecutionCountEstimationPhase.h; path = dfg/DFGStaticExecutionCountEstimationPhase.h; sourceTree = "<group>"; };
                0F50AF3B193E8B3900674EE8 /* DFGStructureClobberState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGStructureClobberState.h; path = dfg/DFGStructureClobberState.h; sourceTree = "<group>"; };
                                0FEC84D81BDACDAC0080FF74 /* B3Opcode.h */,
                                0FEC84D91BDACDAC0080FF74 /* B3Origin.cpp */,
                                0FEC84DA1BDACDAC0080FF74 /* B3Origin.h */,
+                               0F4C91651C29F4F2004341A6 /* B3OriginDump.h */,
                                0FEC84DB1BDACDAC0080FF74 /* B3PatchpointSpecial.cpp */,
                                0FEC84DC1BDACDAC0080FF74 /* B3PatchpointSpecial.h */,
                                0FEC84DD1BDACDAC0080FF74 /* B3PatchpointValue.cpp */,
                                969A07230ED1CE3300F1F681 /* BytecodeGenerator.h in Headers */,
                                7094C4DF1AE439530041A2EE /* BytecodeIntrinsicRegistry.h in Headers */,
                                0F2DD80B1AB3D85800BBB8E8 /* BytecodeKills.h in Headers */,
+                               0F4C91661C29F4F2004341A6 /* B3OriginDump.h in Headers */,
                                C2FCAE1317A9C24E0034C735 /* BytecodeLivenessAnalysis.h in Headers */,
                                0F666EC0183566F900D017F1 /* BytecodeLivenessAnalysisInlines.h in Headers */,
                                6514F21918B3E1670098FF8B /* Bytecodes.h in Headers */,
index 2f5f80c..e2ab3c2 100644 (file)
@@ -108,13 +108,13 @@ void BasicBlock::dump(PrintStream& out) const
     out.print(dumpPrefix, m_index);
 }
 
-void BasicBlock::deepDump(PrintStream& out) const
+void BasicBlock::deepDump(const Procedure& proc, PrintStream& out) const
 {
     out.print("BB", *this, ": ; frequency = ", m_frequency, "\n");
     if (predecessors().size())
         out.print("  Predecessors: ", pointerListDump(predecessors()), "\n");
     for (Value* value : *this)
-        out.print("    ", B3::deepDump(value), "\n");
+        out.print("    ", B3::deepDump(proc, value), "\n");
 }
 
 } } // namespace JSC::B3
index 68b29a7..f6888ca 100644 (file)
@@ -114,7 +114,7 @@ public:
     double frequency() const { return m_frequency; }
 
     void dump(PrintStream&) const;
-    void deepDump(PrintStream&) const;
+    void deepDump(const Procedure&, PrintStream&) const;
 
 private:
     friend class BlockInsertionSet;
@@ -132,26 +132,28 @@ private:
 
 class DeepBasicBlockDump {
 public:
-    DeepBasicBlockDump(const BasicBlock* block)
-        : m_block(block)
+    DeepBasicBlockDump(const Procedure& proc, const BasicBlock* block)
+        : m_proc(proc)
+        , m_block(block)
     {
     }
 
     void dump(PrintStream& out) const
     {
         if (m_block)
-            m_block->deepDump(out);
+            m_block->deepDump(m_proc, out);
         else
             out.print("<null>");
     }
 
 private:
+    const Procedure& m_proc;
     const BasicBlock* m_block;
 };
 
-inline DeepBasicBlockDump deepDump(const BasicBlock* block)
+inline DeepBasicBlockDump deepDump(const Procedure& proc, const BasicBlock* block)
 {
-    return DeepBasicBlockDump(block);
+    return DeepBasicBlockDump(proc, block);
 }
 
 } } // namespace JSC::B3
index 4069b9f..9396632 100644 (file)
@@ -102,7 +102,7 @@ public:
                     continue;
                 m_insts.append(Vector<Inst>());
                 if (verbose)
-                    dataLog("Lowering ", deepDump(m_value), ":\n");
+                    dataLog("Lowering ", deepDump(m_procedure, m_value), ":\n");
                 lower();
                 if (verbose) {
                     for (Inst& inst : m_insts.last())
@@ -2147,7 +2147,7 @@ private:
             break;
         }
 
-        dataLog("FATAL: could not lower ", deepDump(m_value), "\n");
+        dataLog("FATAL: could not lower ", deepDump(m_procedure, m_value), "\n");
         RELEASE_ASSERT_NOT_REACHED();
     }
 
index 242c669..a11b57f 100644 (file)
@@ -47,6 +47,7 @@ public:
 
     const void* data() const { return m_data; }
 
+    // You should avoid using this. Use OriginDump instead.
     void dump(PrintStream&) const;
     
 private:
diff --git a/Source/JavaScriptCore/b3/B3OriginDump.h b/Source/JavaScriptCore/b3/B3OriginDump.h
new file mode 100644 (file)
index 0000000..38534e9
--- /dev/null
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ */
+
+#ifndef B3OriginDump_h
+#define B3OriginDump_h
+
+#if ENABLE(B3_JIT)
+
+#include "B3Origin.h"
+#include "B3Procedure.h"
+
+namespace JSC { namespace B3 {
+
+class OriginDump {
+public:
+    OriginDump(const Procedure& proc, Origin origin)
+        : m_proc(proc)
+        , m_origin(origin)
+    {
+    }
+
+    void dump(PrintStream& out) const
+    {
+        m_proc.printOrigin(out, m_origin);
+    }
+
+private:
+    const Procedure& m_proc;
+    Origin m_origin;
+};
+
+} } // namespace JSC::B3
+
+#endif // ENABLE(B3_JIT)
+
+#endif // B3OriginDump_h
+
index 81cedcc..c693c14 100644 (file)
@@ -52,6 +52,14 @@ Procedure::~Procedure()
 {
 }
 
+void Procedure::printOrigin(PrintStream& out, Origin origin) const
+{
+    if (m_originPrinter)
+        m_originPrinter->run(out, origin);
+    else
+        out.print(origin);
+}
+
 BasicBlock* Procedure::addBlock(double frequency)
 {
     std::unique_ptr<BasicBlock> block(new BasicBlock(m_blocks.size(), frequency));
@@ -126,7 +134,7 @@ void Procedure::invalidateCFG()
 void Procedure::dump(PrintStream& out) const
 {
     for (BasicBlock* block : *this)
-        out.print(deepDump(block));
+        out.print(deepDump(*this, block));
     if (m_byproducts->count())
         out.print(*m_byproducts);
 }
index 4f3ff84..70e40f0 100644 (file)
@@ -39,6 +39,7 @@
 #include <wtf/HashSet.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/PrintStream.h>
+#include <wtf/SharedTask.h>
 #include <wtf/TriState.h>
 #include <wtf/Vector.h>
 
@@ -60,6 +61,16 @@ public:
     JS_EXPORT_PRIVATE Procedure();
     JS_EXPORT_PRIVATE ~Procedure();
 
+    template<typename Callback>
+    void setOriginPrinter(Callback&& callback)
+    {
+        m_originPrinter = createSharedTask<void(PrintStream&, Origin)>(
+            std::forward<Callback>(callback));
+    }
+
+    // Usually you use this via OriginDump, though it's cool to use it directly.
+    void printOrigin(PrintStream& out, Origin origin) const;
+
     JS_EXPORT_PRIVATE BasicBlock* addBlock(double frequency = 1);
     
     template<typename ValueType, typename... Arguments>
@@ -261,6 +272,7 @@ private:
     const char* m_lastPhaseName;
     std::unique_ptr<OpaqueByproducts> m_byproducts;
     std::unique_ptr<Air::Code> m_code;
+    RefPtr<SharedTask<void(PrintStream&, Origin)>> m_originPrinter;
 };
 
 } } // namespace JSC::B3
index 8d768d9..30db709 100644 (file)
@@ -32,6 +32,7 @@
 #include "B3CCallValue.h"
 #include "B3ControlValue.h"
 #include "B3MemoryValue.h"
+#include "B3OriginDump.h"
 #include "B3ProcedureInlines.h"
 #include "B3StackSlotValue.h"
 #include "B3UpsilonValue.h"
@@ -99,7 +100,7 @@ void Value::dumpChildren(CommaPrinter& comma, PrintStream& out) const
         out.print(comma, pointerDump(child));
 }
 
-void Value::deepDump(PrintStream& out) const
+void Value::deepDump(const Procedure& proc, PrintStream& out) const
 {
     out.print(m_type, " ", *this, " = ", m_opcode);
 
@@ -108,7 +109,7 @@ void Value::deepDump(PrintStream& out) const
     dumpChildren(comma, out);
 
     if (m_origin)
-        out.print(comma, m_origin);
+        out.print(comma, OriginDump(proc, m_origin));
 
     dumpMeta(comma, out);
 
index 6377222..3edb2f7 100644 (file)
@@ -85,7 +85,7 @@ public:
     void replaceWithNop();
 
     void dump(PrintStream&) const;
-    void deepDump(PrintStream&) const;
+    void deepDump(const Procedure&, PrintStream&) const;
 
     // This is how you cast Values. For example, if you want to do something provided that we have a
     // ArgumentRegValue, you can do:
@@ -314,26 +314,28 @@ public:
 
 class DeepValueDump {
 public:
-    DeepValueDump(const Value* value)
-        : m_value(value)
+    DeepValueDump(const Procedure& proc, const Value* value)
+        : m_proc(proc)
+        , m_value(value)
     {
     }
 
     void dump(PrintStream& out) const
     {
         if (m_value)
-            m_value->deepDump(out);
+            m_value->deepDump(m_proc, out);
         else
             out.print("<null>");
     }
 
 private:
+    const Procedure& m_proc;
     const Value* m_value;
 };
 
-inline DeepValueDump deepDump(const Value* value)
+inline DeepValueDump deepDump(const Procedure& proc, const Value* value)
 {
-    return DeepValueDump(value);
+    return DeepValueDump(proc, value);
 }
 
 } } // namespace JSC::B3
index 965a3c0..34ce0a5 100644 (file)
@@ -67,6 +67,18 @@ StackSlotValue* Output::lockedStackSlot(size_t bytes)
     return m_block->appendNew<StackSlotValue>(m_proc, origin(), bytes, StackSlotKind::Locked);
 }
 
+LValue Output::bitNot(LValue value)
+{
+    return m_block->appendNew<B3::Value>(m_proc, B3::BitXor, origin(),
+        value,
+        m_block->appendIntConstant(m_proc, origin(), value->type(), -1));
+}
+
+LValue Output::logicalNot(LValue value)
+{
+    return m_block->appendNew<B3::Value>(m_proc, B3::Equal, origin(), value, int32Zero);
+}
+
 LValue Output::load(TypedPointer pointer, LType type)
 {
     LValue load = m_block->appendNew<MemoryValue>(m_proc, Load, type, origin(), pointer.value());
index 2e8e171..1da6b43 100644 (file)
@@ -153,6 +153,7 @@ public:
     LValue aShr(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::SShr, origin(), left, castToInt32(right)); }
     LValue lShr(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::ZShr, origin(), left, castToInt32(right)); }
     LValue bitNot(LValue);
+    LValue logicalNot(LValue);
 
     LValue ctlz32(LValue operand) { return m_block->appendNew<B3::Value>(m_proc, B3::Clz, origin(), operand); }
     LValue addWithOverflow32(LValue left, LValue right) { CRASH(); }
@@ -335,44 +336,17 @@ public:
     LValue lessThanOrEqual(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::LessEqual, origin(), left, right); }
 
     LValue doubleEqual(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::Equal, origin(), left, right); }
+    LValue doubleEqualOrUnordered(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::EqualOrUnordered, origin(), left, right); }
     LValue doubleNotEqualOrUnordered(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::NotEqual, origin(), left, right); }
     LValue doubleLessThan(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::LessThan, origin(), left, right); }
     LValue doubleLessThanOrEqual(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::LessEqual, origin(), left, right); }
     LValue doubleGreaterThan(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::GreaterThan, origin(), left, right); }
     LValue doubleGreaterThanOrEqual(LValue left, LValue right) { return m_block->appendNew<B3::Value>(m_proc, B3::GreaterEqual, origin(), left, right); }
-    LValue doubleNotEqualAndOrdered(LValue left, LValue right)
-    {
-        LValue equalOrUnordered = m_block->appendNew<B3::Value>(m_proc, B3::EqualOrUnordered, origin(), left, right);
-        return bitXor(equalOrUnordered, int32One);
-    }
-    LValue doubleLessThanOrUnordered(LValue left, LValue right)
-    {
-        return m_block->appendNew<B3::Value>(
-            m_proc, B3::Equal, origin(),
-            m_block->appendNew<B3::Value>(m_proc, B3::GreaterEqual, origin(), left, right),
-            int32Zero);
-    }
-    LValue doubleLessThanOrEqualOrUnordered(LValue left, LValue right)
-    {
-        return m_block->appendNew<B3::Value>(
-            m_proc, B3::Equal, origin(),
-            m_block->appendNew<B3::Value>(m_proc, B3::GreaterThan, origin(), left, right),
-            int32Zero);
-    }
-    LValue doubleGreaterThanOrUnordered(LValue left, LValue right)
-    {
-        return m_block->appendNew<B3::Value>(
-            m_proc, B3::Equal, origin(),
-            m_block->appendNew<B3::Value>(m_proc, B3::LessEqual, origin(), left, right),
-            int32Zero);
-    }
-    LValue doubleGreaterThanOrEqualOrUnordered(LValue left, LValue right)
-    {
-        return m_block->appendNew<B3::Value>(
-            m_proc, B3::Equal, origin(),
-            m_block->appendNew<B3::Value>(m_proc, B3::LessThan, origin(), left, right),
-            int32Zero);
-    }
+    LValue doubleNotEqualAndOrdered(LValue left, LValue right) { return logicalNot(doubleEqualOrUnordered(left, right)); }
+    LValue doubleLessThanOrUnordered(LValue left, LValue right) { return logicalNot(doubleGreaterThanOrEqual(left, right)); }
+    LValue doubleLessThanOrEqualOrUnordered(LValue left, LValue right) { return logicalNot(doubleGreaterThan(left, right)); }
+    LValue doubleGreaterThanOrUnordered(LValue left, LValue right) { return logicalNot(doubleLessThanOrEqual(left, right)); }
+    LValue doubleGreaterThanOrEqualOrUnordered(LValue left, LValue right) { return logicalNot(doubleLessThan(left, right)); }
 
     LValue isZero32(LValue value) { return m_block->appendNew<B3::Value>(m_proc, B3::Equal, origin(), value, int32Zero); }
     LValue notZero32(LValue value) { return m_block->appendNew<B3::Value>(m_proc, B3::NotEqual, origin(), value, int32Zero); }
@@ -540,13 +514,6 @@ inline void Output::addIncomingToPhi(LValue phi, ValueFromBlock value, Params...
     addIncomingToPhi(phi, theRest...);
 }
 
-inline LValue Output::bitNot(LValue value)
-{
-    return m_block->appendNew<B3::Value>(m_proc, B3::BitXor, origin(),
-        value,
-        m_block->appendIntConstant(m_proc, origin(), value->type(), -1));
-}
-
 inline LValue Output::bitCast(LValue value, LType type)
 {
     ASSERT_UNUSED(type, type == int64 || type == doubleType);
index 2224fb5..f304524 100644 (file)
@@ -2700,7 +2700,7 @@ private:
         
         speculate(
             BadIndexingType, jsValueValue(cell), 0,
-            m_out.bitNot(isArrayType(cell, m_node->arrayMode())));
+            m_out.logicalNot(isArrayType(cell, m_node->arrayMode())));
     }
 
     void compileGetTypedArrayByteOffset()
@@ -4899,7 +4899,7 @@ private:
     
     void compileLogicalNot()
     {
-        setBoolean(m_out.bitNot(boolify(m_node->child1())));
+        setBoolean(m_out.logicalNot(boolify(m_node->child1())));
     }
 
     void compileCallOrConstruct()
@@ -6168,7 +6168,7 @@ private:
         LValue constructor = lowCell(m_node->child2());
         LValue hasInstance = lowJSValue(m_node->child3());
 
-        setBoolean(m_out.bitNot(m_out.equal(m_out.constInt32(0), vmCall(m_out.int32, m_out.operation(operationInstanceOfCustom), m_callFrame, value, constructor, hasInstance))));
+        setBoolean(m_out.logicalNot(m_out.equal(m_out.constInt32(0), vmCall(m_out.int32, m_out.operation(operationInstanceOfCustom), m_callFrame, value, constructor, hasInstance))));
     }
     
     void compileCountExecution()
@@ -7631,7 +7631,7 @@ private:
         case DoubleRepUse:
             return m_out.doubleNotEqualAndOrdered(lowDouble(edge), m_out.doubleZero);
         case ObjectOrOtherUse:
-            return m_out.bitNot(
+            return m_out.logicalNot(
                 equalNullOrUndefined(
                     edge, CellCaseSpeculatesObject, SpeculateNullOrUndefined,
                     ManualOperandSpeculation));
@@ -9104,7 +9104,7 @@ private:
     {
         if (LValue proven = isProvenValue(type, SpecMisc))
             return proven;
-        return m_out.bitNot(isNotMisc(value));
+        return m_out.logicalNot(isNotMisc(value));
     }
     
     LValue isNotBoolean(LValue jsValue, SpeculatedType type = SpecFullTop)
@@ -9119,7 +9119,7 @@ private:
     {
         if (LValue proven = isProvenValue(type, SpecBoolean))
             return proven;
-        return m_out.bitNot(isNotBoolean(jsValue));
+        return m_out.logicalNot(isNotBoolean(jsValue));
     }
     LValue unboxBoolean(LValue jsValue)
     {
@@ -9410,7 +9410,7 @@ private:
     
     LValue isNotType(LValue cell, JSType type)
     {
-        return m_out.bitNot(isType(cell, type));
+        return m_out.logicalNot(isType(cell, type));
     }
     
     void speculateObject(Edge edge, LValue cell)
index 30628bc..524ad68 100644 (file)
@@ -144,6 +144,7 @@ public:
     LValue aShr(LValue left, LValue right) { return buildAShr(m_builder, left, right); } // arithmetic = signed
     LValue lShr(LValue left, LValue right) { return buildLShr(m_builder, left, right); } // logical = unsigned
     LValue bitNot(LValue value) { return buildNot(m_builder, value); }
+    LValue logicalNot(LValue value) { return bitNot(value); }
     
     LValue insertElement(LValue vector, LValue element, LValue index) { return buildInsertElement(m_builder, vector, element, index); }
 
index 3cbb5b6..fc0eceb 100644 (file)
@@ -72,6 +72,11 @@ State::State(Graph& graph)
 
 #if FTL_USES_B3
     proc = std::make_unique<Procedure>();
+
+    proc->setOriginPrinter(
+        [this] (PrintStream& out, B3::Origin origin) {
+            out.print("DFG:", bitwise_cast<Node*>(origin.data()));
+        });
 #endif // FTL_USES_B3
 }
 
index 855a0e7..5e05dfa 100644 (file)
@@ -1,3 +1,17 @@
+2015-12-22  Filip Pizlo  <fpizlo@apple.com>
+
+        FTL B3 does not logicalNot correctly
+        https://bugs.webkit.org/show_bug.cgi?id=152512
+
+        Reviewed by Saam Barati.
+
+        This change introduces yet another use of SharedTask in JSC. While doing this, I noticed that
+        SharedTask::run() always demands that whatever arguments the callback takes, they must be
+        passed as rvalue references. This was a clear misuse of perfect forwarding. This change makes
+        SharedTask's approach to forwarding match what we were already doing in ScopedLambda.
+
+        * wtf/SharedTask.h:
+
 2015-12-20  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [SOUP] Performs DNS prefetch when a proxy is configured (information leak)
index 5805918..6023665 100644 (file)
@@ -65,7 +65,7 @@ public:
     SharedTask() { }
     virtual ~SharedTask() { }
 
-    virtual ResultType run(ArgumentTypes&&...) = 0;
+    virtual ResultType run(ArgumentTypes...) = 0;
 };
 
 // This is a utility class that allows you to create a SharedTask subclass using a lambda. Usually,
@@ -85,9 +85,9 @@ public:
     }
 
 private:
-    ResultType run(ArgumentTypes&&... arguments) override
+    ResultType run(ArgumentTypes... arguments) override
     {
-        return m_functor(std::forward<ArgumentTypes>(arguments)...);
+        return m_functor(arguments...);
     }
 
     Functor m_functor;