From 84eae1a413a97f8dcc412cecf907016cb2ff7918 Mon Sep 17 00:00:00 2001
From: Xun Li <xun@fb.com>
Date: Thu, 2 Jul 2020 14:28:13 -0700
Subject: [PATCH] [Bolt] Improve coding style for runtime lib related code

Summary:
Reading through the LLVM coding standard again, realized a few places where I didn't follow the standard when coding. Addressing them:
1. prefer static functions over functions in unnamed namespace.
2. #include as little as possible in headers
3. Have vtable anchors.

(cherry picked from FBD22353046)
---
 bolt/runtime/hugify.cpp                               |  8 +++-----
 bolt/src/RuntimeLibs/HugifyRuntimeLibrary.cpp         |  1 +
 .../src/RuntimeLibs/InstrumentationRuntimeLibrary.cpp |  1 +
 bolt/src/RuntimeLibs/RuntimeLibrary.cpp               |  3 +++
 bolt/src/RuntimeLibs/RuntimeLibrary.h                 | 11 ++++++++++-
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/bolt/runtime/hugify.cpp b/bolt/runtime/hugify.cpp
index 3050907c468e..09468a27e7a9 100644
--- a/bolt/runtime/hugify.cpp
+++ b/bolt/runtime/hugify.cpp
@@ -24,14 +24,13 @@ extern void (*__bolt_hugify_init_ptr)();
 extern uint64_t __hot_start;
 extern uint64_t __hot_end;
 
-namespace {
 #ifdef MADV_HUGEPAGE
 /// Starting from character at \p buf, find the longest consecutive sequence
 /// of digits (0-9) and convert it to uint32_t. The converted value
 /// is put into \p ret. \p end marks the end of the buffer to avoid buffer
 /// overflow. The function \returns whether a valid uint32_t value is found.
 /// \p buf will be updated to the next character right after the digits.
-bool scanUInt32(const char *&buf, const char *end, uint32_t &ret) {
+static bool scanUInt32(const char *&buf, const char *end, uint32_t &ret) {
   uint64_t result = 0;
   const char *oldBuf = buf;
   while (buf < end && ((*buf) >= '0' && (*buf) <= '9')) {
@@ -47,7 +46,7 @@ bool scanUInt32(const char *&buf, const char *end, uint32_t &ret) {
 
 /// Check whether the kernel supports THP by checking the kernel version.
 /// Only fb kernel 5.2 and latter supports it.
-bool has_pagecache_thp_support() {
+static bool has_pagecache_thp_support() {
   struct utsname u;
   int ret = __uname(&u);
   if (ret) {
@@ -92,7 +91,7 @@ bool has_pagecache_thp_support() {
   return nums[1] > 2 || nums[4] >= 5;
 }
 
-void hugify_for_old_kernel(uint8_t *from, uint8_t *to) {
+static void hugify_for_old_kernel(uint8_t *from, uint8_t *to) {
   size_t size = to - from;
 
   uint8_t *mem = reinterpret_cast<uint8_t *>(
@@ -164,7 +163,6 @@ extern "C" void __bolt_hugify_self_impl() {
   }
 #endif
 }
-} // anonymous namespace
 
 /// This is hooking ELF's entry, it needs to save all machine state.
 extern "C" __attribute((naked)) void __bolt_hugify_self() {
diff --git a/bolt/src/RuntimeLibs/HugifyRuntimeLibrary.cpp b/bolt/src/RuntimeLibs/HugifyRuntimeLibrary.cpp
index 7512b1531f5c..592043f41672 100644
--- a/bolt/src/RuntimeLibs/HugifyRuntimeLibrary.cpp
+++ b/bolt/src/RuntimeLibs/HugifyRuntimeLibrary.cpp
@@ -9,6 +9,7 @@
 
 #include "HugifyRuntimeLibrary.h"
 #include "BinaryFunction.h"
+#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
 
 using namespace llvm;
 using namespace bolt;
diff --git a/bolt/src/RuntimeLibs/InstrumentationRuntimeLibrary.cpp b/bolt/src/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
index 9efa9f759064..6baa14005848 100644
--- a/bolt/src/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
+++ b/bolt/src/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
@@ -11,6 +11,7 @@
 
 #include "InstrumentationRuntimeLibrary.h"
 #include "BinaryFunction.h"
+#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
 
 using namespace llvm;
 using namespace bolt;
diff --git a/bolt/src/RuntimeLibs/RuntimeLibrary.cpp b/bolt/src/RuntimeLibs/RuntimeLibrary.cpp
index 9ab794697f6b..ab6336e49fce 100644
--- a/bolt/src/RuntimeLibs/RuntimeLibrary.cpp
+++ b/bolt/src/RuntimeLibs/RuntimeLibrary.cpp
@@ -11,6 +11,7 @@
 
 #include "RuntimeLibrary.h"
 #include "Utils.h"
+#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Support/Path.h"
 
@@ -20,6 +21,8 @@
 using namespace llvm;
 using namespace bolt;
 
+void RuntimeLibrary::anchor() {}
+
 std::string RuntimeLibrary::getLibPath(StringRef ToolPath,
                                        StringRef LibFileName) {
   auto Dir = llvm::sys::path::parent_path(ToolPath);
diff --git a/bolt/src/RuntimeLibs/RuntimeLibrary.h b/bolt/src/RuntimeLibs/RuntimeLibrary.h
index e23262725b2c..e6ad250f9b53 100644
--- a/bolt/src/RuntimeLibs/RuntimeLibrary.h
+++ b/bolt/src/RuntimeLibs/RuntimeLibrary.h
@@ -15,16 +15,25 @@
 #ifndef LLVM_TOOLS_LLVM_BOLT_LINKRUNTIME_H
 #define LLVM_TOOLS_LLVM_BOLT_LINKRUNTIME_H
 
-#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
+#include <llvm/ADT/StringRef.h>
+#include <vector>
 
 namespace llvm {
 class MCStreamer;
 
+namespace orc {
+class ExecutionSession;
+class RTDyldObjectLinkingLayer;
+} // namespace orc
+
 namespace bolt {
 
 class BinaryContext;
 
 class RuntimeLibrary {
+  // vtable anchor.
+  virtual void anchor();
+
 public:
   virtual ~RuntimeLibrary() = default;