Changeset 286802 in webkit

Timestamp:

Dec 9, 2021, 1:49:45 PM (5 years ago)

Author:

sbarati@apple.com

Message:

Procedure::setNeedsPCToOriginMap should call Code::forcePreservationOfB3Origins
​https://bugs.webkit.org/show_bug.cgi?id=234093

Reviewed by Yusuke Suzuki.

We need to do this to ensure the sampling profiler works in FTL.
The reason this was sometimes working was Air::Code's constructor
was looking at Procedure's m_needsPCToOriginMap before it was initialized,
in its constructor. This is because Procedure was constructing Code
before all its fields were initialized. This patch fixes that bug to
construct Code after Procedure has all its fields initialized.

• b3/B3Procedure.cpp:

(JSC::B3::Procedure::Procedure):
(JSC::B3::Procedure::setNeedsPCToOriginMap):

• b3/B3Procedure.h:

(JSC::B3::Procedure::setNeedsPCToOriginMap): Deleted.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• b3/B3Procedure.cpp (modified) (2 diffs)
• b3/B3Procedure.h (modified) (1 diff)
• b3/air/AirCode.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-12-09  Saam Barati  <sbarati@apple.com>
+
+        Procedure::setNeedsPCToOriginMap should call Code::forcePreservationOfB3Origins
+        https://bugs.webkit.org/show_bug.cgi?id=234093
+
+        Reviewed by Yusuke Suzuki.
+
+        We need to do this to ensure the sampling profiler works in FTL.
+        The reason this was sometimes working was Air::Code's constructor
+        was looking at Procedure's m_needsPCToOriginMap before it was initialized,
+        in its constructor. This is because Procedure was constructing Code
+        before all its fields were initialized. This patch fixes that bug to
+        construct Code after Procedure has all its fields initialized.
+
+        * b3/B3Procedure.cpp:
+        (JSC::B3::Procedure::Procedure):
+        (JSC::B3::Procedure::setNeedsPCToOriginMap):
+        * b3/B3Procedure.h:
+        (JSC::B3::Procedure::setNeedsPCToOriginMap): Deleted.
+
 2021-12-09  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/b3/B3Procedure.cpp

@@ -49 +49 @@
     , m_lastPhaseName("initial")
     , m_byproducts(makeUnique<OpaqueByproducts>())
-    , m_code(new Air::Code(*this))
-{
+{
+    // Initialize all our fields before constructing Air::Code since
+    // it looks into our fields.
+    m_code = std::unique_ptr<Air::Code>(new Air::Code(*this));
     m_code->setNumEntrypoints(m_numEntrypoints);
 }
@@ -487 +489 @@
 }
 
+void Procedure::setNeedsPCToOriginMap()
+{ 
+    m_needsPCToOriginMap = true;
+    m_code->forcePreservationOfB3Origins();
+}
+
 } } // namespace JSC::B3
 

trunk/Source/JavaScriptCore/b3/B3Procedure.h

@@ -276 +276 @@
     JS_EXPORT_PRIVATE RegisterSet mutableFPRs();
 
-    void setNeedsPCToOriginMap() { m_needsPCToOriginMap = true; }
+    void setNeedsPCToOriginMap();
     bool needsPCToOriginMap() { return m_needsPCToOriginMap; }
 

trunk/Source/JavaScriptCore/b3/air/AirCode.cpp

@@ -57 +57 @@
     : m_proc(proc)
     , m_cfg(new CFG(*this))
-    , m_preserveB3Origins(proc.needsPCToOriginMap() || Options::dumpAirGraphAtEachPhase() || Options::dumpFTLDisassembly())
+    , m_preserveB3Origins(Options::dumpAirGraphAtEachPhase() || Options::dumpFTLDisassembly())
     , m_lastPhaseName("initial")
     , m_defaultPrologueGenerator(createSharedTask<PrologueGeneratorFunction>(&defaultPrologueGenerator))