<div dir="ltr">Hans, this is the last of the commits I'd like in 3.6 to address compile time issues. This one hasn't fully made it through the build bots though, but I didn't want to forget to put it on your radar assuming it survives.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 28, 2015 at 1:52 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: chandlerc<br>
Date: Wed Jan 28 03:52:14 2015<br>
New Revision: 227300<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=227300&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=227300&view=rev</a><br>
Log:<br>
[LPM] Rip all of ManagedStatic and ThreadLocal out of the pretty stack<br>
tracing code.<br>
<br>
Managed static was just insane overhead for this. We took memory fences<br>
and external function calls in every path that pushed a pretty stack<br>
frame. This includes a multitude of layers setting up and tearing down<br>
passes, the parser in Clang, everywhere. For the regression test suite<br>
or low-overhead JITs, this was contributing to really significant<br>
overhead.<br>
<br>
Even the LLVM ThreadLocal is really overkill here because it uses<br>
pthread_{set,get}_specific logic, and has careful code to both allocate<br>
and delete the thread local data. We don't actually want any of that,<br>
and this code in particular has problems coping with deallocation. What<br>
we want is a single TLS pointer that is valid to use during global<br>
construction and during global destruction, any time we want. That is<br>
exactly what every host compiler and OS we use has implemented for<br>
a long time, and what was standardized in C++11. Even though not all of<br>
our host compilers support the thread_local keyword, we can directly use<br>
the platform-specific keywords to get the minimal functionality needed.<br>
Provided this limited trial survives the build bots, I will move this to<br>
Compiler.h so it is more widely available as a light weight if limited<br>
alternative to the ThreadLocal class. Many thanks to David Majnemer for<br>
helping me think through the implications across platforms and craft the<br>
MSVC-compatible syntax.<br>
<br>
The end result is *substantially* faster. When running llc in a tight<br>
loop over a small IR file targeting the aarch64 backend, this improves<br>
its performance by over 10% for me. It also seems likely to fix the<br>
remaining regressions seen by JIT users with threading enabled.<br>
<br>
This may actually have more impact on real-world compile times due to<br>
the use of the pretty stack tracing utility throughout the rest of Clang<br>
or LLVM, but I've not collected any detailed measurements.<br>
<br>
Modified:<br>
llvm/trunk/lib/Support/PrettyStackTrace.cpp<br>
<br>
Modified: llvm/trunk/lib/Support/PrettyStackTrace.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=227300&r1=227299&r2=227300&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=227300&r1=227299&r2=227300&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/PrettyStackTrace.cpp (original)<br>
+++ llvm/trunk/lib/Support/PrettyStackTrace.cpp Wed Jan 28 03:52:14 2015<br>
@@ -16,9 +16,7 @@<br>
#include "llvm-c/Core.h"<br>
#include "llvm/ADT/SmallString.h"<br>
#include "llvm/Config/config.h" // Get autoconf configuration settings<br>
-#include "llvm/Support/ManagedStatic.h"<br>
#include "llvm/Support/Signals.h"<br>
-#include "llvm/Support/ThreadLocal.h"<br>
#include "llvm/Support/Watchdog.h"<br>
#include "llvm/Support/raw_ostream.h"<br>
<br>
@@ -28,7 +26,18 @@<br>
<br>
using namespace llvm;<br>
<br>
-static ManagedStatic<sys::ThreadLocal<const PrettyStackTraceEntry> > PrettyStackTraceHead;<br>
+// We need a thread local pointer to manage the stack of our stack trace<br>
+// objects, but we *really* cannot tolerate destructors running and do not want<br>
+// to pay any overhead of synchronizing. As a consequence, we use a raw<br>
+// thread-local variable. Some day, we should be able to use a limited subset<br>
+// of C++11's thread_local, but compilers aren't up for it today.<br>
+// FIXME: This should be moved to a Compiler.h abstraction.<br>
+#ifdef _MSC_VER // MSVC supports this with a __declspec.<br>
+static __declspec(thread) const PrettyStackTraceEntry<br>
+ *PrettyStackTraceHead = nullptr;<br>
+#else // Clang, GCC, and all compatible compilers tend to use __thread.<br>
+static __thread const PrettyStackTraceEntry *PrettyStackTraceHead = nullptr;<br>
+#endif<br>
<br>
static unsigned PrintStack(const PrettyStackTraceEntry *Entry, raw_ostream &OS){<br>
unsigned NextID = 0;<br>
@@ -46,12 +55,12 @@ static unsigned PrintStack(const PrettyS<br>
/// PrintCurStackTrace - Print the current stack trace to the specified stream.<br>
static void PrintCurStackTrace(raw_ostream &OS) {<br>
// Don't print an empty trace.<br>
- if (!PrettyStackTraceHead->get()) return;<br>
+ if (!PrettyStackTraceHead) return;<br>
<br>
// If there are pretty stack frames registered, walk and emit them.<br>
OS << "Stack dump:\n";<br>
<br>
- PrintStack(PrettyStackTraceHead->get(), OS);<br>
+ PrintStack(PrettyStackTraceHead, OS);<br>
OS.flush();<br>
}<br>
<br>
@@ -101,26 +110,14 @@ static void CrashHandler(void *) {<br>
<br>
PrettyStackTraceEntry::PrettyStackTraceEntry() {<br>
// Link ourselves.<br>
- NextEntry = PrettyStackTraceHead->get();<br>
- PrettyStackTraceHead->set(this);<br>
+ NextEntry = PrettyStackTraceHead;<br>
+ PrettyStackTraceHead = this;<br>
}<br>
<br>
PrettyStackTraceEntry::~PrettyStackTraceEntry() {<br>
- // Do nothing if PrettyStackTraceHead is uninitialized. This can only happen<br>
- // if a shutdown occurred after we created the PrettyStackTraceEntry. That<br>
- // does occur in the following idiom:<br>
- //<br>
- // PrettyStackTraceProgram X(...);<br>
- // llvm_shutdown_obj Y;<br>
- //<br>
- // Without this check, we may end up removing ourselves from the stack trace<br>
- // after PrettyStackTraceHead has already been destroyed.<br>
- if (!PrettyStackTraceHead.isConstructed())<br>
- return;<br>
-<br>
- assert(PrettyStackTraceHead->get() == this &&<br>
+ assert(PrettyStackTraceHead == this &&<br>
"Pretty stack trace entry destruction is out of order");<br>
- PrettyStackTraceHead->set(getNextEntry());<br>
+ PrettyStackTraceHead = getNextEntry();<br>
}<br>
<br>
void PrettyStackTraceString::print(raw_ostream &OS) const {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>