Skip to content

Commit

Permalink
[Bolt] Improve coding style for runtime lib related code
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
lxfind authored and maksfb committed Jul 2, 2020
1 parent e233dec commit 84eae1a
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 6 deletions.
8 changes: 3 additions & 5 deletions bolt/runtime/hugify.cpp
Expand Up @@ -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')) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 *>(
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions bolt/src/RuntimeLibs/HugifyRuntimeLibrary.cpp
Expand Up @@ -9,6 +9,7 @@

#include "HugifyRuntimeLibrary.h"
#include "BinaryFunction.h"
#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"

using namespace llvm;
using namespace bolt;
Expand Down
1 change: 1 addition & 0 deletions bolt/src/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
Expand Up @@ -11,6 +11,7 @@

#include "InstrumentationRuntimeLibrary.h"
#include "BinaryFunction.h"
#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"

using namespace llvm;
using namespace bolt;
Expand Down
3 changes: 3 additions & 0 deletions bolt/src/RuntimeLibs/RuntimeLibrary.cpp
Expand Up @@ -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"

Expand All @@ -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);
Expand Down
11 changes: 10 additions & 1 deletion bolt/src/RuntimeLibs/RuntimeLibrary.h
Expand Up @@ -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;

Expand Down

0 comments on commit 84eae1a

Please sign in to comment.