[llvm] [BOLT][NFC] Refactor BC::createBinaryContext for #81346 (PR #87172)
Amir Ayupov via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 30 18:45:41 PDT 2024
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/87172
None
>From 30dcb38e8401ec076ff99a8d898aa3a83ff7d7ff Mon Sep 17 00:00:00 2001
From: Amir Ayupov <amir.aupov at gmail.com>
Date: Sat, 30 Mar 2024 18:36:46 -0700
Subject: [PATCH] [BOLT][NFC] Refactor BC::createBinaryContext for #81346
---
bolt/include/bolt/Core/BinaryContext.h | 3 +-
bolt/lib/Core/BinaryContext.cpp | 39 ++++++++++++-----------
bolt/lib/Rewrite/DWARFRewriter.cpp | 2 +-
bolt/lib/Rewrite/MachORewriteInstance.cpp | 35 ++------------------
bolt/lib/Rewrite/RewriteInstance.cpp | 19 ++++++++++-
bolt/unittests/Core/BinaryContext.cpp | 4 +--
bolt/unittests/Core/MCPlusBuilder.cpp | 4 +--
7 files changed, 48 insertions(+), 58 deletions(-)
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index 741b1a36af86f8..8b1af9e8153925 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -265,7 +265,8 @@ class BinaryContext {
public:
static Expected<std::unique_ptr<BinaryContext>>
- createBinaryContext(const ObjectFile *File, bool IsPIC,
+ createBinaryContext(Triple TheTriple, StringRef InputFileName,
+ SubtargetFeatures *Features, bool IsPIC,
std::unique_ptr<DWARFContext> DwCtx,
JournalingStreams Logger);
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 267f43f65e206e..7b474bc40dac0f 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -162,28 +162,30 @@ BinaryContext::~BinaryContext() {
/// Create BinaryContext for a given architecture \p ArchName and
/// triple \p TripleName.
-Expected<std::unique_ptr<BinaryContext>>
-BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
- std::unique_ptr<DWARFContext> DwCtx,
- JournalingStreams Logger) {
+Expected<std::unique_ptr<BinaryContext>> BinaryContext::createBinaryContext(
+ Triple TheTriple, StringRef InputFileName, SubtargetFeatures *Features,
+ bool IsPIC, std::unique_ptr<DWARFContext> DwCtx, JournalingStreams Logger) {
StringRef ArchName = "";
std::string FeaturesStr = "";
- switch (File->getArch()) {
+ switch (TheTriple.getArch()) {
case llvm::Triple::x86_64:
+ if (Features)
+ return createFatalBOLTError(
+ "x86_64 target does not use SubtargetFeatures");
ArchName = "x86-64";
FeaturesStr = "+nopl";
break;
case llvm::Triple::aarch64:
+ if (Features)
+ return createFatalBOLTError(
+ "AArch64 target does not use SubtargetFeatures");
ArchName = "aarch64";
FeaturesStr = "+all";
break;
case llvm::Triple::riscv64: {
ArchName = "riscv64";
- Expected<SubtargetFeatures> Features = File->getFeatures();
-
- if (auto E = Features.takeError())
- return std::move(E);
-
+ if (!Features)
+ return createFatalBOLTError("RISCV target needs SubtargetFeatures");
// We rely on relaxation for some transformations (e.g., promoting all calls
// to PseudoCALL and then making JITLink relax them). Since the relax
// feature is not stored in the object file, we manually enable it.
@@ -196,12 +198,11 @@ BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
"BOLT-ERROR: Unrecognized machine in ELF file");
}
- auto TheTriple = std::make_unique<Triple>(File->makeTriple());
- const std::string TripleName = TheTriple->str();
+ const std::string TripleName = TheTriple.str();
std::string Error;
const Target *TheTarget =
- TargetRegistry::lookupTarget(std::string(ArchName), *TheTriple, Error);
+ TargetRegistry::lookupTarget(std::string(ArchName), TheTriple, Error);
if (!TheTarget)
return createStringError(make_error_code(std::errc::not_supported),
Twine("BOLT-ERROR: ", Error));
@@ -240,13 +241,13 @@ BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
Twine("BOLT-ERROR: no instruction info for target ", TripleName));
std::unique_ptr<MCContext> Ctx(
- new MCContext(*TheTriple, AsmInfo.get(), MRI.get(), STI.get()));
+ new MCContext(TheTriple, AsmInfo.get(), MRI.get(), STI.get()));
std::unique_ptr<MCObjectFileInfo> MOFI(
TheTarget->createMCObjectFileInfo(*Ctx, IsPIC));
Ctx->setObjectFileInfo(MOFI.get());
// We do not support X86 Large code model. Change this in the future.
bool Large = false;
- if (TheTriple->getArch() == llvm::Triple::aarch64)
+ if (TheTriple.getArch() == llvm::Triple::aarch64)
Large = true;
unsigned LSDAEncoding =
Large ? dwarf::DW_EH_PE_absptr : dwarf::DW_EH_PE_udata4;
@@ -273,7 +274,7 @@ BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
int AsmPrinterVariant = AsmInfo->getAssemblerDialect();
std::unique_ptr<MCInstPrinter> InstructionPrinter(
- TheTarget->createMCInstPrinter(*TheTriple, AsmPrinterVariant, *AsmInfo,
+ TheTarget->createMCInstPrinter(TheTriple, AsmPrinterVariant, *AsmInfo,
*MII, *MRI));
if (!InstructionPrinter)
return createStringError(
@@ -285,8 +286,8 @@ BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
TheTarget->createMCCodeEmitter(*MII, *Ctx));
auto BC = std::make_unique<BinaryContext>(
- std::move(Ctx), std::move(DwCtx), std::move(TheTriple), TheTarget,
- std::string(TripleName), std::move(MCE), std::move(MOFI),
+ std::move(Ctx), std::move(DwCtx), std::make_unique<Triple>(TheTriple),
+ TheTarget, std::string(TripleName), std::move(MCE), std::move(MOFI),
std::move(AsmInfo), std::move(MII), std::move(STI),
std::move(InstructionPrinter), std::move(MIA), nullptr, std::move(MRI),
std::move(DisAsm), Logger);
@@ -296,7 +297,7 @@ BinaryContext::createBinaryContext(const ObjectFile *File, bool IsPIC,
BC->MAB = std::unique_ptr<MCAsmBackend>(
BC->TheTarget->createMCAsmBackend(*BC->STI, *BC->MRI, MCTargetOptions()));
- BC->setFilename(File->getFileName());
+ BC->setFilename(InputFileName);
BC->HasFixedLoadAddress = !IsPIC;
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index 601a2105fc264f..6a6f5708cdf753 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -1685,7 +1685,7 @@ namespace {
std::unique_ptr<BinaryContext>
createDwarfOnlyBC(const object::ObjectFile &File) {
return cantFail(BinaryContext::createBinaryContext(
- &File, false,
+ File.makeTriple(), File.getFileName(), nullptr, false,
DWARFContext::create(File, DWARFContext::ProcessDebugRelocations::Ignore,
nullptr, "", WithColor::defaultErrorHandler,
WithColor::defaultWarningHandler),
diff --git a/bolt/lib/Rewrite/MachORewriteInstance.cpp b/bolt/lib/Rewrite/MachORewriteInstance.cpp
index 0970a0507ebe88..172cb640bf911a 100644
--- a/bolt/lib/Rewrite/MachORewriteInstance.cpp
+++ b/bolt/lib/Rewrite/MachORewriteInstance.cpp
@@ -18,6 +18,7 @@
#include "bolt/Rewrite/BinaryPassManager.h"
#include "bolt/Rewrite/ExecutableFileMemoryManager.h"
#include "bolt/Rewrite/JITLinkLinker.h"
+#include "bolt/Rewrite/RewriteInstance.h"
#include "bolt/RuntimeLibs/InstrumentationRuntimeLibrary.h"
#include "bolt/Utils/Utils.h"
#include "llvm/MC/MCObjectStreamer.h"
@@ -54,37 +55,6 @@ extern cl::opt<unsigned> Verbosity;
namespace llvm {
namespace bolt {
-extern MCPlusBuilder *createX86MCPlusBuilder(const MCInstrAnalysis *,
- const MCInstrInfo *,
- const MCRegisterInfo *,
- const MCSubtargetInfo *);
-extern MCPlusBuilder *createAArch64MCPlusBuilder(const MCInstrAnalysis *,
- const MCInstrInfo *,
- const MCRegisterInfo *,
- const MCSubtargetInfo *);
-
-namespace {
-
-MCPlusBuilder *createMCPlusBuilder(const Triple::ArchType Arch,
- const MCInstrAnalysis *Analysis,
- const MCInstrInfo *Info,
- const MCRegisterInfo *RegInfo,
- const MCSubtargetInfo *STI) {
-#ifdef X86_AVAILABLE
- if (Arch == Triple::x86_64)
- return createX86MCPlusBuilder(Analysis, Info, RegInfo, STI);
-#endif
-
-#ifdef AARCH64_AVAILABLE
- if (Arch == Triple::aarch64)
- return createAArch64MCPlusBuilder(Analysis, Info, RegInfo, STI);
-#endif
-
- llvm_unreachable("architecture unsupported by MCPlusBuilder");
-}
-
-} // anonymous namespace
-
#define DEBUG_TYPE "bolt"
Expected<std::unique_ptr<MachORewriteInstance>>
@@ -103,7 +73,8 @@ MachORewriteInstance::MachORewriteInstance(object::MachOObjectFile *InputFile,
: InputFile(InputFile), ToolPath(ToolPath) {
ErrorAsOutParameter EAO(&Err);
auto BCOrErr = BinaryContext::createBinaryContext(
- InputFile, /* IsPIC */ true, DWARFContext::create(*InputFile),
+ InputFile->makeTriple(), InputFile->getFileName(), nullptr,
+ /* IsPIC */ true, DWARFContext::create(*InputFile),
{llvm::outs(), llvm::errs()});
if (Error E = BCOrErr.takeError()) {
Err = std::move(E);
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 5ca5594117a62f..0c8ee0d417233b 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -268,6 +268,10 @@ namespace bolt {
extern const char *BoltRevision;
+// Weird location for createMCPlusBuilder, but this is here to avoid a
+// cyclic dependency of libCore (its natural place) and libTarget. libRewrite
+// can depend on libTarget, but not libCore. Since libRewrite is the only
+// user of this function, we define it here.
MCPlusBuilder *createMCPlusBuilder(const Triple::ArchType Arch,
const MCInstrAnalysis *Analysis,
const MCInstrInfo *Info,
@@ -345,8 +349,21 @@ RewriteInstance::RewriteInstance(ELFObjectFileBase *File, const int Argc,
Stderr.SetUnbuffered();
LLVM_DEBUG(dbgs().SetUnbuffered());
+ // Read RISCV subtarget features from input file
+ std::unique_ptr<SubtargetFeatures> Features;
+ Triple TheTriple = File->makeTriple();
+ if (TheTriple.getArch() == llvm::Triple::riscv64) {
+ Expected<SubtargetFeatures> FeaturesOrErr = File->getFeatures();
+ if (auto E = FeaturesOrErr.takeError()) {
+ Err = std::move(E);
+ return;
+ } else {
+ Features.reset(new SubtargetFeatures(*FeaturesOrErr));
+ }
+ }
+
auto BCOrErr = BinaryContext::createBinaryContext(
- File, IsPIC,
+ TheTriple, File->getFileName(), Features.get(), IsPIC,
DWARFContext::create(*File, DWARFContext::ProcessDebugRelocations::Ignore,
nullptr, opts::DWPPathName,
WithColor::defaultErrorHandler,
diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp
index 08619eccdd75ab..6737ba27bfcdfe 100644
--- a/bolt/unittests/Core/BinaryContext.cpp
+++ b/bolt/unittests/Core/BinaryContext.cpp
@@ -40,8 +40,8 @@ struct BinaryContextTester : public testing::TestWithParam<Triple::ArchType> {
void initializeBOLT() {
BC = cantFail(BinaryContext::createBinaryContext(
- ObjFile.get(), true, DWARFContext::create(*ObjFile.get()),
- {llvm::outs(), llvm::errs()}));
+ ObjFile->makeTriple(), ObjFile->getFileName(), nullptr, true,
+ DWARFContext::create(*ObjFile.get()), {llvm::outs(), llvm::errs()}));
ASSERT_FALSE(!BC);
}
diff --git a/bolt/unittests/Core/MCPlusBuilder.cpp b/bolt/unittests/Core/MCPlusBuilder.cpp
index daf9f392b82281..2fbfc792e42ee7 100644
--- a/bolt/unittests/Core/MCPlusBuilder.cpp
+++ b/bolt/unittests/Core/MCPlusBuilder.cpp
@@ -50,8 +50,8 @@ struct MCPlusBuilderTester : public testing::TestWithParam<Triple::ArchType> {
void initializeBolt() {
BC = cantFail(BinaryContext::createBinaryContext(
- ObjFile.get(), true, DWARFContext::create(*ObjFile.get()),
- {llvm::outs(), llvm::errs()}));
+ ObjFile->makeTriple(), ObjFile->getFileName(), nullptr, true,
+ DWARFContext::create(*ObjFile.get()), {llvm::outs(), llvm::errs()}));
ASSERT_FALSE(!BC);
BC->initializeTarget(std::unique_ptr<MCPlusBuilder>(
createMCPlusBuilder(GetParam(), BC->MIA.get(), BC->MII.get(),
More information about the llvm-commits
mailing list