Attachment 265719 Details for Bug 151371 – the patch

[patch] the patch

blah.patch (text/plain), 11.82 KB, created by Filip Pizlo on 2015-11-17 17:20:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2015-11-17 17:20:44 PST

Size: 11.82 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 192557)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,50 @@
>+2015-11-17  Filip Pizlo  <fpizlo@apple.com>
>+
>+        B3::generate should separate out the final Air codegen, so that it can be done outside the Graph safepoint
>+        https://bugs.webkit.org/show_bug.cgi?id=151371
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        One of the FTL optimizations is that while the expensive backend is running, we are at a "graph
>+        safepoint" that allows the VM to do GCs and other dangerous and time-sensitive things without
>+        waiting for the compilation thread. While in the safepoint, we cannot do anything that touches
>+        anything other than the backend's state. That means, for example, that we wouldn't be able to run
>+        any of the stackmap generation callbacks, since those need to mess with DFG state.
>+
>+        That means that we need to separate the B3 pipeline into "preperation" and "generation".
>+        Preparation is all of the expensive stuff: all B3 phases, lowering to Air, all Air phases.
>+        Generation is just the part where we turn fully lowered Air into machine code. Generation also
>+        happens to be the part where we call stackmap generation callbacks. In other words, preperation
>+        is exactly the stuff that should go into the graph safepoint, while generation is the stuff that
>+        we want to do after we emerge from the safepoint.
>+
>+        Because the tests were using the higher-level Compilation API, I didn't have to change any test
>+        code. The FTL will not use that high-level API.
>+
>+        * b3/B3Compilation.cpp:
>+        (JSC::B3::Compilation::Compilation):
>+        * b3/B3Generate.cpp:
>+        (JSC::B3::prepareForGeneration):
>+        (JSC::B3::generate):
>+        (JSC::B3::generateToAir):
>+        * b3/B3Generate.h:
>+        * b3/B3LowerToAir.cpp:
>+        (JSC::B3::Air::LowerToAir::LowerToAir):
>+        (JSC::B3::lowerToAir):
>+        * b3/B3LowerToAir.h:
>+        * b3/B3Procedure.cpp:
>+        (JSC::B3::Procedure::Procedure):
>+        * b3/B3Procedure.h:
>+        (JSC::B3::Procedure::takeByproducts):
>+        (JSC::B3::Procedure::code):
>+        * b3/air/AirCode.h:
>+        (JSC::B3::Air::Code::proc):
>+        (JSC::B3::Air::Code::lastPhaseName):
>+        * b3/air/AirGenerate.cpp:
>+        (JSC::B3::Air::prepareForGeneration):
>+        (JSC::B3::Air::generate):
>+        * b3/air/AirGenerate.h:
>+
> 2015-11-17  Filip Pizlo  <fpizlo@apple.com>
> 
>         FTL::State should be able to refer to B3::Procedure
>Index: Source/JavaScriptCore/b3/B3Compilation.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3Compilation.cpp	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3Compilation.cpp	(working copy)
>@@ -31,6 +31,7 @@
> #include "B3Generate.h"
> #include "B3OpaqueByproducts.h"
> #include "B3Procedure.h"
>+#include "B3TimingScope.h"
> #include "CCallHelpers.h"
> #include "JSCInlines.h"
> #include "LinkBuffer.h"
>@@ -39,8 +40,11 @@ namespace JSC { namespace B3 {
> 
> Compilation::Compilation(VM& vm, Procedure& proc, unsigned optLevel)
> {
>+    TimingScope timingScope("Compilation");
>+    
>     CCallHelpers jit(&vm);
>-    generate(proc, jit, optLevel);
>+    prepareForGeneration(proc, optLevel);
>+    generate(proc, jit);
>     LinkBuffer linkBuffer(vm, jit, nullptr);
> 
>     m_codeRef = FINALIZE_CODE(linkBuffer, ("B3::Compilation"));
>Index: Source/JavaScriptCore/b3/B3Generate.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3Generate.cpp	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3Generate.cpp	(working copy)
>@@ -42,16 +42,20 @@
> 
> namespace JSC { namespace B3 {
> 
>-void generate(Procedure& procedure, CCallHelpers& jit, unsigned optLevel)
>+void prepareForGeneration(Procedure& procedure, unsigned optLevel)
> {
>-    TimingScope timingScope("generate");
>+    TimingScope timingScope("prepareForGeneration");
> 
>-    Air::Code code(procedure);
>-    generateToAir(procedure, code, optLevel);
>-    Air::generate(code, jit);
>+    generateToAir(procedure, optLevel);
>+    Air::prepareForGeneration(procedure.code());
> }
> 
>-void generateToAir(Procedure& procedure, Air::Code& code, unsigned optLevel)
>+void generate(Procedure& procedure, CCallHelpers& jit)
>+{
>+    Air::generate(procedure.code(), jit);
>+}
>+
>+void generateToAir(Procedure& procedure, unsigned optLevel)
> {
>     TimingScope timingScope("generateToAir");
>     
>@@ -87,7 +91,7 @@ void generateToAir(Procedure& procedure,
>         dataLog(procedure);
>     }
> 
>-    lowerToAir(procedure, code);
>+    lowerToAir(procedure);
> }
> 
> } } // namespace JSC::B3
>Index: Source/JavaScriptCore/b3/B3Generate.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3Generate.h	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3Generate.h	(working copy)
>@@ -37,14 +37,19 @@ namespace B3 {
> class Procedure;
> namespace Air { class Code; }
> 
>-// This takes a B3::Procedure, optimizes it in-place, and generates it to machine code by first
>-// internally converting to an Air::Code and then generating that.
>-JS_EXPORT_PRIVATE void generate(Procedure&, CCallHelpers&, unsigned optLevel = 1);
>+// This takes a B3::Procedure, optimizes it in-place, lowers it to Air, and prepares the Air for
>+// generation.
>+void prepareForGeneration(Procedure&, unsigned optLevel = 1);
>+
>+// This takes a B3::Procedure that has been prepared for generation (i.e. it has been lowered to Air and
>+// the Air has been prepared for generation) and generates it. This is the equivalent of calling
>+// Air::generate() on the Procedure::code().
>+void generate(Procedure&, CCallHelpers&);
> 
> // This takes a B3::Procedure, optimizes it in-place, and lowers it to Air. You can then generate
>-// the Air to machine code using Air::generate(). Note that an Air::Code will have pointers into the
>-// B3::Procedure, so you need to ensure that the B3::Procedure outlives the Air::Code.
>-void generateToAir(Procedure&, Air::Code&, unsigned optLevel = 1);
>+// the Air to machine code using Air::prepareForGeneration() and Air::generate() on the Procedure's
>+// code().
>+void generateToAir(Procedure&, unsigned optLevel = 1);
> 
> } } // namespace JSC::B3
> 
>Index: Source/JavaScriptCore/b3/B3LowerToAir.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3LowerToAir.cpp	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3LowerToAir.cpp	(working copy)
>@@ -61,12 +61,12 @@ const bool verbose = false;
> 
> class LowerToAir {
> public:
>-    LowerToAir(Procedure& procedure, Code& code)
>+    LowerToAir(Procedure& procedure)
>         : m_valueToTmp(procedure.values().size())
>         , m_blockToBlock(procedure.size())
>         , m_useCounts(procedure)
>         , m_procedure(procedure)
>-        , m_code(code)
>+        , m_code(procedure.code())
>     {
>     }
> 
>@@ -1745,10 +1745,10 @@ private:
> 
> } // anonymous namespace
> 
>-void lowerToAir(Procedure& procedure, Code& code)
>+void lowerToAir(Procedure& procedure)
> {
>     PhaseScope phaseScope(procedure, "lowerToAir");
>-    LowerToAir lowerToAir(procedure, code);
>+    LowerToAir lowerToAir(procedure);
>     lowerToAir.run();
> }
> 
>Index: Source/JavaScriptCore/b3/B3LowerToAir.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3LowerToAir.h	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3LowerToAir.h	(working copy)
>@@ -35,7 +35,7 @@ namespace Air { class Code; }
> 
> // This lowers the current B3 procedure to an Air code.
> 
>-void lowerToAir(Procedure&, Air::Code&);
>+void lowerToAir(Procedure&);
> 
> } } // namespace JSC::B3
> 
>Index: Source/JavaScriptCore/b3/B3Procedure.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/B3Procedure.cpp	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3Procedure.cpp	(working copy)
>@@ -28,6 +28,7 @@
> 
> #if ENABLE(B3_JIT)
> 
>+#include "AirCode.h"
> #include "B3BasicBlockInlines.h"
> #include "B3BasicBlockUtils.h"
> #include "B3BlockWorklist.h"
>@@ -40,6 +41,7 @@ namespace JSC { namespace B3 {
> Procedure::Procedure()
>     : m_lastPhaseName("initial")
>     , m_byproducts(std::make_unique<OpaqueByproducts>())
>+    , m_code(new Air::Code(*this))
> {
> }
> 
>Index: Source/JavaScriptCore/b3/B3Procedure.h
>===================================================================
>--- Source/JavaScriptCore/b3/B3Procedure.h	(revision 192557)
>+++ Source/JavaScriptCore/b3/B3Procedure.h	(working copy)
>@@ -45,6 +45,8 @@ class BlockInsertionSet;
> class OpaqueByproducts;
> class Value;
> 
>+namespace Air { class Code; }
>+
> class Procedure {
>     WTF_MAKE_NONCOPYABLE(Procedure);
>     WTF_MAKE_FAST_ALLOCATED;
>@@ -218,6 +220,8 @@ public:
>     // that API, then you don't have to worry about this.
>     std::unique_ptr<OpaqueByproducts> takeByproducts() { return WTF::move(m_byproducts); }
> 
>+    Air::Code& code() { return *m_code; }
>+
> private:
>     friend class BlockInsertionSet;
>     
>@@ -228,6 +232,7 @@ private:
>     Vector<size_t> m_valueIndexFreeList;
>     const char* m_lastPhaseName;
>     std::unique_ptr<OpaqueByproducts> m_byproducts;
>+    std::unique_ptr<Air::Code> m_code;
> };
> 
> } } // namespace JSC::B3
>Index: Source/JavaScriptCore/b3/air/AirCode.h
>===================================================================
>--- Source/JavaScriptCore/b3/air/AirCode.h	(revision 192557)
>+++ Source/JavaScriptCore/b3/air/AirCode.h	(working copy)
>@@ -50,7 +50,6 @@ class Code {
>     WTF_MAKE_NONCOPYABLE(Code);
>     WTF_MAKE_FAST_ALLOCATED;
> public:
>-    Code(Procedure&);
>     ~Code();
> 
>     Procedure& proc() { return m_proc; }
>@@ -299,6 +298,10 @@ public:
>     const char* lastPhaseName() const { return m_lastPhaseName; }
> 
> private:
>+    friend class ::JSC::B3::Procedure;
>+    
>+    Code(Procedure&);
>+
>     Procedure& m_proc; // Some meta-data, like byproducts, is stored in the Procedure.
>     Vector<std::unique_ptr<StackSlot>> m_stackSlots;
>     Vector<std::unique_ptr<BasicBlock>> m_blocks;
>Index: Source/JavaScriptCore/b3/air/AirGenerate.cpp
>===================================================================
>--- Source/JavaScriptCore/b3/air/AirGenerate.cpp	(revision 192557)
>+++ Source/JavaScriptCore/b3/air/AirGenerate.cpp	(working copy)
>@@ -46,9 +46,9 @@
> 
> namespace JSC { namespace B3 { namespace Air {
> 
>-void generate(Code& code, CCallHelpers& jit)
>+void prepareForGeneration(Code& code)
> {
>-    TimingScope timingScope("Air::generate");
>+    TimingScope timingScope("Air::prepareForGeneration");
>     
>     // We don't expect the incoming code to have predecessors computed.
>     code.resetReachability();
>@@ -106,8 +106,11 @@ void generate(Code& code, CCallHelpers& 
>         dataLog("Air after ", code.lastPhaseName(), ", before generation:\n");
>         dataLog(code);
>     }
>+}
> 
>-    TimingScope codeGenTimingScope("Air::generate backend");
>+void generate(Code& code, CCallHelpers& jit)
>+{
>+    TimingScope timingScope("Air::generate");
> 
>     // And now, we generate code.
>     jit.emitFunctionPrologue();
>Index: Source/JavaScriptCore/b3/air/AirGenerate.h
>===================================================================
>--- Source/JavaScriptCore/b3/air/AirGenerate.h	(revision 192557)
>+++ Source/JavaScriptCore/b3/air/AirGenerate.h	(working copy)
>@@ -37,9 +37,11 @@ namespace B3 { namespace Air {
> class Code;
> 
> // This takes an Air::Code that hasn't had any stack allocation and optionally hasn't had any
>-// register allocation and does both of those things, and then generates the code using the given
>-// CCallHelpers instance. Note that this may call callbacks in the supplied code as it is
>-// generating.
>+// register allocation and does both of those things.
>+void prepareForGeneration(Code&);
>+
>+// This generates the code using the given CCallHelpers instance. Note that this may call callbacks
>+// in the supplied code as it is generating.
> void generate(Code&, CCallHelpers&);
> 
> } } } // namespace JSC::B3::Air

Flags:

benjamin: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 151371: 265719