[PATCH] D39761: [Support] Make llvm::Error faster

[Zachary Turner via Phabricator via llvm-commits]

zturner created this revision.

  Whenever LLVM_ENABLE_ABI_BREAKING_CHECKS is enabled, which
  is usually the case for example when asserts are enabled,
  Error's destructor does some additional checking to make sure
  that that it does not represent an error condition and that it
  was checked.
  
  However, this is -- by definition -- not the likely codepath.
  Some profiling shows that at least with some compilers, simply
  calling assertIsChecked -- in a release build with full
  optimizations -- can account for up to 15% of the entire
  runtime of the program, even though this function should almost
  literally be a no-op.
  
  The problem is that the assertIsChecked function can be considered
  too big to inline depending on the compiler's inliner.  Since it's
  unlikely to ever need to failure path though, we can move it out
  of line and force it to not be inlined, so that the fast path
  can be inlined.
  
  In my test (using lld to link clang with CMAKE_BUILD_TYPE=Release
  and LLVM_ENABLE_ASSERTIONS=ON), this reduces link time from 27
  seconds to 23.5 seconds, which is a solid 15% gain.


https://reviews.llvm.org/D39761

Files:
  llvm/include/llvm/Support/Error.h


Index: llvm/include/llvm/Support/Error.h
===================================================================
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -246,20 +246,31 @@
   }
 
 private:
-  void assertIsChecked() {
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
-    if (!getChecked() || getPtr()) {
-      dbgs() << "Program aborted due to an unhandled Error:\n";
-      if (getPtr())
-        getPtr()->log(dbgs());
-      else
-        dbgs()
-            << "Error value was Success. (Note: Success values must still be "
-               "checked prior to being destroyed).\n";
-      abort();
-    }
-#endif
+  // assertIsChecked() happens very frequently, but under normal circumstances
+  // is supposed to be a no-op.  So we want it to be inlined, but having a bunch
+  // of debug prints can cause the function to be too large for inlining.  So
+  // we hint the compiler by making sure the unlikely path doesn't get inlined,
+  // which should guarantee that the fast path does get inlined.
+  LLVM_ATTRIBUTE_NOINLINE
+  void fatalUncheckedError() const {
+    dbgs() << "Program aborted due to an unhandled Error:\n";
+    if (getPtr())
+      getPtr()->log(dbgs());
+    else
+      dbgs() << "Error value was Success. (Note: Success values must still be "
+                "checked prior to being destroyed).\n";
+    abort();
+  }
+
+  void assertIsChecked() {
+    if (LLVM_UNLIKELY(!getChecked() || getPtr()))
+      fatalUncheckedError();
+  }
+#else
+  void assertIsChecked() {
   }
+#endif
 
   ErrorInfoBase *getPtr() const {
     return reinterpret_cast<ErrorInfoBase*>(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39761.121980.patch
Type: text/x-patch
Size: 1619 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171107/ba3d5c02/attachment.bin>