[BOLT] Enable standalone build #97130
Conversation
RossComputerGuy
requested review from
aaupov,
maksfb,
rafaelauler,
ayermolo and
dcci
as code owners
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-bolt @llvm/pr-subscribers-backend-risc-v Author: Tristan Ross (RossComputerGuy) ChangesContinue from #87196 as author did not have much time, I have taken over working on this PR. We would like to have this so it'll be easier to package for Nix. Full diff: https://github.com/llvm/llvm-project/pull/97130.diff 8 Files Affected:
diff --git a/bolt/CMakeLists.txt b/bolt/CMakeLists.txt
index 74907ad118d12f..ce8e7b9bb2b931 100644
--- a/bolt/CMakeLists.txt
+++ b/bolt/CMakeLists.txt
@@ -1,6 +1,17 @@
+cmake_minimum_required(VERSION 3.20.0)
+
set(LLVM_SUBPROJECT_TITLE "BOLT")
-include(ExternalProject)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+ set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
+ NO_POLICY_SCOPE)
+
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+ project(bolt)
+ set(BOLT_BUILT_STANDALONE TRUE)
+endif()
set(BOLT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
set(BOLT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
@@ -9,6 +20,39 @@ set(CMAKE_CXX_STANDARD 17)
# Add path for custom modules.
list(INSERT CMAKE_MODULE_PATH 0 "${BOLT_SOURCE_DIR}/cmake/modules")
+# standalone build, copied from clang
+if(BOLT_BUILT_STANDALONE)
+ set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
+ set(CMAKE_CXX_STANDARD_REQUIRED YES)
+ set(CMAKE_CXX_EXTENSIONS NO)
+
+ if(NOT MSVC_IDE)
+ set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
+ CACHE BOOL "Enable assertions")
+ # Assertions should follow llvm-config's.
+ mark_as_advanced(LLVM_ENABLE_ASSERTIONS)
+ endif()
+
+ find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_DIR}")
+ list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
+
+ set(LLVM_MAIN_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../llvm" CACHE PATH "Path to LLVM source tree")
+ find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
+ NO_DEFAULT_PATH)
+
+ # They are used as destination of target generators.
+ set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
+ set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
+
+ include(AddLLVM)
+ include(TableGen)
+ include_directories(${LLVM_INCLUDE_DIRS})
+
+ set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin )
+ set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+ set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+endif() # standalone
+
# Determine default set of targets to build -- the intersection of
# those BOLT supports and those LLVM is targeting.
set(BOLT_TARGETS_TO_BUILD_all "AArch64;X86;RISCV")
@@ -94,6 +138,8 @@ if (BOLT_ENABLE_RUNTIME)
if(CMAKE_SYSROOT)
list(APPEND extra_args -DCMAKE_SYSROOT=${CMAKE_SYSROOT})
endif()
+
+ include(ExternalProject)
ExternalProject_Add(bolt_rt
SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/runtime"
STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-stamps
diff --git a/bolt/lib/Target/AArch64/CMakeLists.txt b/bolt/lib/Target/AArch64/CMakeLists.txt
index be03e247aa96b1..ba7be426203bdd 100644
--- a/bolt/lib/Target/AArch64/CMakeLists.txt
+++ b/bolt/lib/Target/AArch64/CMakeLists.txt
@@ -4,6 +4,39 @@ set(LLVM_LINK_COMPONENTS
AArch64Desc
)
+if(BOLT_BUILT_STANDALONE)
+ # tablegen, copied from llvm/lib/Target/AAarch64/CMakeLists.txt
+ set(LLVM_TARGET_DEFINITIONS ${LLVM_MAIN_SRC_DIR}/lib/Target/AArch64/AArch64.td)
+ list(APPEND LLVM_TABLEGEN_FLAGS -I ${LLVM_MAIN_SRC_DIR}/lib/Target/AArch64)
+ tablegen(LLVM AArch64GenAsmMatcher.inc -gen-asm-matcher)
+ tablegen(LLVM AArch64GenAsmWriter.inc -gen-asm-writer)
+ tablegen(LLVM AArch64GenAsmWriter1.inc -gen-asm-writer -asmwriternum=1)
+ tablegen(LLVM AArch64GenCallingConv.inc -gen-callingconv)
+ tablegen(LLVM AArch64GenDAGISel.inc -gen-dag-isel)
+ tablegen(LLVM AArch64GenDisassemblerTables.inc -gen-disassembler)
+ tablegen(LLVM AArch64GenFastISel.inc -gen-fast-isel)
+ tablegen(LLVM AArch64GenGlobalISel.inc -gen-global-isel)
+ tablegen(LLVM AArch64GenO0PreLegalizeGICombiner.inc -gen-global-isel-combiner
+ -combiners="AArch64O0PreLegalizerCombiner")
+ tablegen(LLVM AArch64GenPreLegalizeGICombiner.inc -gen-global-isel-combiner
+ -combiners="AArch64PreLegalizerCombiner")
+ tablegen(LLVM AArch64GenPostLegalizeGICombiner.inc -gen-global-isel-combiner
+ -combiners="AArch64PostLegalizerCombiner")
+ tablegen(LLVM AArch64GenPostLegalizeGILowering.inc -gen-global-isel-combiner
+ -combiners="AArch64PostLegalizerLowering")
+ tablegen(LLVM AArch64GenInstrInfo.inc -gen-instr-info)
+ tablegen(LLVM AArch64GenMCCodeEmitter.inc -gen-emitter)
+ tablegen(LLVM AArch64GenMCPseudoLowering.inc -gen-pseudo-lowering)
+ tablegen(LLVM AArch64GenRegisterBank.inc -gen-register-bank)
+ tablegen(LLVM AArch64GenRegisterInfo.inc -gen-register-info)
+ tablegen(LLVM AArch64GenSubtargetInfo.inc -gen-subtarget)
+ tablegen(LLVM AArch64GenSystemOperands.inc -gen-searchable-tables)
+ tablegen(LLVM AArch64GenExegesis.inc -gen-exegesis)
+
+ add_public_tablegen_target(AArch64CommonTableGen)
+ include_directories(${CMAKE_CURRENT_BINARY_DIR})
+endif()
+
add_llvm_library(LLVMBOLTTargetAArch64
AArch64MCPlusBuilder.cpp
diff --git a/bolt/lib/Target/RISCV/CMakeLists.txt b/bolt/lib/Target/RISCV/CMakeLists.txt
index 7f955760632008..6016a13a1b0829 100644
--- a/bolt/lib/Target/RISCV/CMakeLists.txt
+++ b/bolt/lib/Target/RISCV/CMakeLists.txt
@@ -4,6 +4,37 @@ set(LLVM_LINK_COMPONENTS
RISCVDesc
)
+if(BOLT_BUILT_STANDALONE)
+ # tablegen, copied from llvm/lib/Target/RISCV/CMakeLists.txt
+ set(LLVM_TARGET_DEFINITIONS ${LLVM_MAIN_SRC_DIR}/lib/Target/RISCV/RISCV.td)
+ list(APPEND LLVM_TABLEGEN_FLAGS -I ${LLVM_MAIN_SRC_DIR}/lib/Target/RISCV)
+ tablegen(LLVM RISCVGenAsmMatcher.inc -gen-asm-matcher)
+ tablegen(LLVM RISCVGenAsmWriter.inc -gen-asm-writer)
+ tablegen(LLVM RISCVGenCompressInstEmitter.inc -gen-compress-inst-emitter)
+ tablegen(LLVM RISCVGenMacroFusion.inc -gen-macro-fusion-pred)
+ tablegen(LLVM RISCVGenDAGISel.inc -gen-dag-isel)
+ tablegen(LLVM RISCVGenDisassemblerTables.inc -gen-disassembler)
+ tablegen(LLVM RISCVGenInstrInfo.inc -gen-instr-info)
+ tablegen(LLVM RISCVGenMCCodeEmitter.inc -gen-emitter)
+ tablegen(LLVM RISCVGenMCPseudoLowering.inc -gen-pseudo-lowering)
+ tablegen(LLVM RISCVGenRegisterBank.inc -gen-register-bank)
+ tablegen(LLVM RISCVGenRegisterInfo.inc -gen-register-info)
+ tablegen(LLVM RISCVGenSearchableTables.inc -gen-searchable-tables)
+ tablegen(LLVM RISCVGenSubtargetInfo.inc -gen-subtarget)
+
+ set(LLVM_TARGET_DEFINITIONS ${LLVM_MAIN_SRC_DIR}/lib/Target/RISCV/RISCVGISel.td)
+ tablegen(LLVM RISCVGenGlobalISel.inc -gen-global-isel)
+ tablegen(LLVM RISCVGenO0PreLegalizeGICombiner.inc -gen-global-isel-combiner
+ -combiners="RISCVO0PreLegalizerCombiner")
+ tablegen(LLVM RISCVGenPreLegalizeGICombiner.inc -gen-global-isel-combiner
+ -combiners="RISCVPreLegalizerCombiner")
+ tablegen(LLVM RISCVGenPostLegalizeGICombiner.inc -gen-global-isel-combiner
+ -combiners="RISCVPostLegalizerCombiner")
+
+ add_public_tablegen_target(RISCVCommonTableGen)
+ include_directories(${CMAKE_CURRENT_BINARY_DIR})
+endif()
+
add_llvm_library(LLVMBOLTTargetRISCV
RISCVMCPlusBuilder.cpp
diff --git a/bolt/lib/Target/X86/CMakeLists.txt b/bolt/lib/Target/X86/CMakeLists.txt
index 2b769bc7e7f5ce..4527c454012379 100644
--- a/bolt/lib/Target/X86/CMakeLists.txt
+++ b/bolt/lib/Target/X86/CMakeLists.txt
@@ -5,6 +5,32 @@ set(LLVM_LINK_COMPONENTS
X86Desc
)
+if(BOLT_BUILT_STANDALONE)
+ # tablegen, copied from llvm/lib/Target/X86/CMakeLists.txt
+ set(LLVM_TARGET_DEFINITIONS ${LLVM_MAIN_SRC_DIR}/lib/Target/X86/X86.td)
+ list(APPEND LLVM_TABLEGEN_FLAGS -I ${LLVM_MAIN_SRC_DIR}/lib/Target/X86)
+ tablegen(LLVM X86GenAsmMatcher.inc -gen-asm-matcher)
+ tablegen(LLVM X86GenAsmWriter.inc -gen-asm-writer)
+ tablegen(LLVM X86GenAsmWriter1.inc -gen-asm-writer -asmwriternum=1)
+ tablegen(LLVM X86GenCallingConv.inc -gen-callingconv)
+ tablegen(LLVM X86GenDAGISel.inc -gen-dag-isel)
+ tablegen(LLVM X86GenDisassemblerTables.inc -gen-disassembler)
+ tablegen(LLVM X86GenCompressEVEXTables.inc -gen-x86-compress-evex-tables)
+ tablegen(LLVM X86GenExegesis.inc -gen-exegesis)
+ tablegen(LLVM X86GenFastISel.inc -gen-fast-isel)
+ tablegen(LLVM X86GenGlobalISel.inc -gen-global-isel)
+ tablegen(LLVM X86GenInstrInfo.inc -gen-instr-info
+ -instr-info-expand-mi-operand-info=0)
+ tablegen(LLVM X86GenMnemonicTables.inc -gen-x86-mnemonic-tables -asmwriternum=1)
+ tablegen(LLVM X86GenRegisterBank.inc -gen-register-bank)
+ tablegen(LLVM X86GenRegisterInfo.inc -gen-register-info)
+ tablegen(LLVM X86GenSubtargetInfo.inc -gen-subtarget)
+ tablegen(LLVM X86GenFoldTables.inc -gen-x86-fold-tables -asmwriternum=1)
+
+ add_public_tablegen_target(X86CommonTableGen)
+ include_directories(${CMAKE_CURRENT_BINARY_DIR})
+endif()
+
add_llvm_library(LLVMBOLTTargetX86
X86MCPlusBuilder.cpp
X86MCSymbolizer.cpp
diff --git a/bolt/lib/Utils/CMakeLists.txt b/bolt/lib/Utils/CMakeLists.txt
index d1403314274bd6..79623b33f4e816 100644
--- a/bolt/lib/Utils/CMakeLists.txt
+++ b/bolt/lib/Utils/CMakeLists.txt
@@ -1,15 +1,43 @@
+find_first_existing_vc_file("${LLVM_MAIN_SRC_DIR}" llvm_vc)
+find_first_existing_vc_file("${BOLT_SOURCE_DIR}" bolt_vc)
+
+# The VC revision include that we want to generate.
+set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
+
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
+
+if(llvm_vc AND LLVM_APPEND_VC_REV)
+ set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
+endif()
+
+# Create custom target to generate the VC revision include.
+add_custom_command(OUTPUT "${version_inc}"
+ DEPENDS "${llvm_vc}" "${bolt_vc}" "${generate_vcs_version_script}"
+ COMMAND ${CMAKE_COMMAND} "-DNAMES=BOLT"
+ "-DHEADER_FILE=${version_inc}"
+ "-DBOLT_SOURCE_DIR=${BOLT_SOURCE_DIR}"
+ "-DLLVM_VC_REPOSITORY=${llvm_vc_repository}"
+ "-DLLVM_VC_REVISION=${llvm_vc_revision}"
+ "-DLLVM_FORCE_VC_REVISION=${LLVM_FORCE_VC_REVISION}"
+ "-DLLVM_FORCE_VC_REPOSITORY=${LLVM_FORCE_VC_REPOSITORY}"
+ -P "${generate_vcs_version_script}")
+
+# Mark the generated header as being generated.
+set_source_files_properties("${version_inc}"
+ PROPERTIES GENERATED TRUE
+ HEADER_FILE_ONLY TRUE)
+
+include_directories(${CMAKE_CURRENT_BINARY_DIR})
+
add_llvm_library(LLVMBOLTUtils
CommandLineOpts.cpp
Utils.cpp
-
+ ${version_inc}
DISABLE_LLVM_LINK_LLVM_DYLIB
LINK_LIBS
${LLVM_PTHREAD_LIB}
- DEPENDS
- llvm_vcsrevision_h
-
LINK_COMPONENTS
Support
)
diff --git a/bolt/lib/Utils/CommandLineOpts.cpp b/bolt/lib/Utils/CommandLineOpts.cpp
index 41c89bc8aeba4e..7a398cf5073d6e 100644
--- a/bolt/lib/Utils/CommandLineOpts.cpp
+++ b/bolt/lib/Utils/CommandLineOpts.cpp
@@ -11,7 +11,7 @@
//===----------------------------------------------------------------------===//
#include "bolt/Utils/CommandLineOpts.h"
-#include "llvm/Support/VCSRevision.h"
+#include "VCSVersion.inc"
using namespace llvm;
diff --git a/bolt/runtime/CMakeLists.txt b/bolt/runtime/CMakeLists.txt
index 6a65f80fb9079f..fb395e481aba6d 100644
--- a/bolt/runtime/CMakeLists.txt
+++ b/bolt/runtime/CMakeLists.txt
@@ -16,12 +16,10 @@ add_library(bolt_rt_instr STATIC
instr.cpp
${CMAKE_CURRENT_BINARY_DIR}/config.h
)
-set_target_properties(bolt_rt_instr PROPERTIES ARCHIVE_OUTPUT_DIRECTORY "${LLVM_LIBRARY_DIR}")
add_library(bolt_rt_hugify STATIC
hugify.cpp
${CMAKE_CURRENT_BINARY_DIR}/config.h
)
-set_target_properties(bolt_rt_hugify PROPERTIES ARCHIVE_OUTPUT_DIRECTORY "${LLVM_LIBRARY_DIR}")
set(BOLT_RT_FLAGS
-ffreestanding
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.h
index d4aa0fe99078e1..6cc22af601fdb7 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.h
@@ -13,7 +13,6 @@
#ifndef LLVM_LIB_TARGET_RISCV_MCTARGETDESC_RISCVMCTARGETDESC_H
#define LLVM_LIB_TARGET_RISCV_MCTARGETDESC_RISCVMCTARGETDESC_H
-#include "llvm/Config/config.h"
#include "llvm/MC/MCTargetOptions.h"
#include "llvm/Support/DataTypes.h"
#include <memory>
|
| tablegen(LLVM AArch64GenDisassemblerTables.inc -gen-disassembler) | ||
| tablegen(LLVM AArch64GenFastISel.inc -gen-fast-isel) | ||
| tablegen(LLVM AArch64GenGlobalISel.inc -gen-global-isel) | ||
| tablegen(LLVM AArch64GenO0PreLegalizeGICombiner.inc -gen-global-isel-combiner |
There was a problem hiding this comment.
I'm not sure BOLT needs these .inc files, but I'm fine if they're included transitively
There was a problem hiding this comment.
You can remove lines where -gen contains `global-isel' . All your potential code generation goes through the DAG.
There was a problem hiding this comment.
BOLT has no code generation.
There was a problem hiding this comment.
So we're good to remove the entire block in bolt/lib/Target/Aarch64/CMakeLists.txt in the if(BOLT_BUILD_STANDALONE)?
There was a problem hiding this comment.
So we're good to remove the entire block [...]?
No; what the above comments are saying is that some of these tablegen invocations are unnecessary.
However, bolt does have some target-specific code living in bolt/lib/Target/*/*MCPlusBuilder.cpp which requires these headers. So far as I can see this requires re-running tablegen in the standalone build which is not ideal but seems a pragmatic step towards getting a standalone build working.
These are the set of tablegen invocations which I think are necessary. If you delete the wrong ones you'd expect to get a build failure with missing include files.
aarch64 needs:
tablegen(LLVM AArch64GenInstrInfo.inc -gen-instr-info)
tablegen(LLVM AArch64GenRegisterInfo.inc -gen-register-info)
tablegen(LLVM AArch64GenSystemOperands.inc -gen-searchable-tables)
x86:
tablegen(LLVM X86GenInstrInfo.inc -gen-instr-info -instr-info-expand-mi-operand-info=0)
tablegen(LLVM X86GenMnemonicTables.inc -gen-x86-mnemonic-tables -asmwriternum=1)
tablegen(LLVM X86GenRegisterInfo.inc -gen-register-info)
tablegen(LLVM X86GenSubtargetInfo.inc -gen-subtarget)
riscv:
tablegen(LLVM RISCVGenInstrInfo.inc -gen-instr-info)
tablegen(LLVM RISCVGenRegisterInfo.inc -gen-register-info)
There was a problem hiding this comment.
Alright, I finally found the time and reduced it down to that list.
Haven't had a chance to test it yet. Only a rebase from the previous author of the PR (#87196). Instructions and stuff remain the same as the previous attempt hence me mentioning it. From the previous PR:
|
RossComputerGuy
force-pushed
the
bolt-standalone
branch
4 times, most recently
from
f9150f9 to
72aff3d
Compare
|
I just tested this with the Nixpkgs PR, it seems to work. If possible, would be great to have this in LLVM 19. |
|
Good way to test this is to copy the |
|
Dropping the RISC-V change and will be done later to prevent CI fails. |
RossComputerGuy
force-pushed
the
bolt-standalone
branch
from
72aff3d to
071252f
Compare
RossComputerGuy
force-pushed
the
bolt-standalone
branch
from
071252f to
c73fa91
Compare
|
Pulled the RISC-V change out into #98790. |
|
I had to redo the runtime CMake change, now bolt is working as expected in Nix and should pass CI. |
|
@aaupov Test plan script (set cp -r cmake $src/cmake
cp -r bolt $src/bolt
cp -r third-party $src/third-party
cp -r llvm $src/llvm
cd $src/build
cmake .. $src/bolt
make # or ninja |
RossComputerGuy
force-pushed
the
bolt-standalone
branch
5 times, most recently
from
878894c to
b340029
Compare
RossComputerGuy
force-pushed
the
bolt-standalone
branch
2 times, most recently
from
a31201d to
ecc4478
Compare
Goes in hand with llvm#97130, split out to figure out CI fails. Should just build whatever subprojects utilize the `RISCVMCTargetDesc.h` header and it should build & test just like normal. Co-authored-by: pca006132 <[email protected]>
| std::string RuntimeLibrary::getLibPath(StringRef ToolPath, | ||
| StringRef LibFileName) { | ||
| std::string ByTool = getLibPathByToolPath(ToolPath, LibFileName); | ||
| if (llvm::sys::fs::exists(ByTool)) { | ||
| return ByTool; | ||
| } | ||
|
|
||
| std::string ByInstalled = getLibPathByInstalled(LibFileName); | ||
| if (llvm::sys::fs::exists(ByInstalled)) { | ||
| return ByInstalled; | ||
| } | ||
|
|
||
| errs() << "BOLT-ERROR: library not found: " << ByTool << " or " << ByInstalled | ||
| << "\n"; | ||
| exit(1); | ||
| } | ||
|
|
There was a problem hiding this comment.
All usages of the string in command line option RuntimeHugifyLib (and the equivalent instrumentation lib one) go through RuntimeLibrary::getLibPath(), but I don't see any check here that matches against "LibFileName" alone. So I think this will still fail if the user provides a full path in RuntimeHugifyLib. We need to first try to match against "LibFileName" alone and only try the additional mechanisms (getLibPathByToolPath, getLibPathByInstalled) after this initial try failed.
There was a problem hiding this comment.
I think we can just throw a fs exists check on top of getLibPath and return the lib file name when that results in true.
RossComputerGuy
force-pushed
the
bolt-standalone
branch
2 times, most recently
from
93cf85b to
f1f0750
Compare
| return std::string(LibPath); | ||
| } | ||
|
|
||
| std::string RuntimeLibrary::getLibPathByInstalled(StringRef LibFileName) { | ||
| SmallString<128> LibPath = | ||
| llvm::sys::path::root_path(CMAKE_INSTALL_FULL_LIBDIR); |
There was a problem hiding this comment.
I still don't understand what's your goal here with a call to "root_path"... this will essentially wipe out any folders in CMAKE_INSTALL_FULL_LIBDIR and use only root "/" in unix or "C:" in Windows.
I think at this point you just want to use CMAKE_INSTALL_FULL_LIBDIR as is?
There was a problem hiding this comment.
I didn't know it would accept the string as it and not without a constructor or method.
There was a problem hiding this comment.
I fixed it to use CMAKE_INSTALL_FULL_LIBDIR without root_path.
|
My work so far has been mainly to steer this in the direction of not breaking the normal build, but I can't verify that the actual goal (the standalone build) is working at all -- in fact, I'm not even sure why is that a thing, and why a builder that is only interested in packaging BOLT wouldn't just build it with: and then I tried that and got a install folder with the bare minimum to run BOLT: So what exactly are we getting by having a standalone CMakeFile? I also tried running cmake directly on llvm-project/bolt with this patch applied, but it fails with: |
I'm kinda new with maintaining LLVM in Nix but afaik it comes down to code deduplicating and managing install size. I could get someone who's on the Nix LLVM team to give a better explanation as to why we need standalone builds.
Interesting, I don't get that error in my testing nor with Nix. |
I help maintain a cross-platform distribution (conda-forge.org), and this would be very helpful to us as well. Here are some reasons why we want to ship llvm as individual (though interdependent) components rather than monolithically:
The first one is a hard constraint that's almost impossible to work around; we don't have a company to pay for CI, so we're stuck with what builds on public CI. The second one is very important at scale (millions of users, billions of downloads) as well. To get a feeling for what I'm talking about, here's a simplified view of our current split:
Note that these are generally locked to the same version, so we're not asking for arbitrary mixing and matching, just being able to build bolt against an existing |
|
Thanks for the context @h-vetinari . I got enough to be able to create my test, and it works nicely, thanks for your work @RossComputerGuy In my test, I:
Then I run " The only remaining concern I have then is that I don't think the |
Yeah, I'll remove the root dir call and just use the string. I haven't had a chance to make that change but I will be able to in a few hours. |
RossComputerGuy
force-pushed
the
bolt-standalone
branch
2 times, most recently
from
283cae1 to
d76ebcd
Compare
RossComputerGuy
force-pushed
the
bolt-standalone
branch
from
d76ebcd to
62cf63d
Compare
|
@RossComputerGuy Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
| return std::string(LibPath); | ||
| } | ||
|
|
||
| std::string RuntimeLibrary::getLibPathByInstalled(StringRef LibFileName) { | ||
| SmallString<128> LibPath(CMAKE_INSTALL_FULL_LIBDIR); |
There was a problem hiding this comment.
This embeds a full absolute path into the bolt binary. That doesn't seem great, since it means that that library has to be at that particular path. Other tools instead require a fixed relative path from executable to runtime library (with an overridable setting).
See Driver::GetResourcesPath in clang/lib/Driver/Driver.cpp (and its caller, in the same file), and ToolChain::getCompilerRTPath() in clang/lib/Driver/ToolChain.cpp.
There was a problem hiding this comment.
This particular method is meant only for when the binary is installed. getLibPathByToolPath gets the path relative to the binary. getLibPath calls both methods to ensure it finds the right library.
There was a problem hiding this comment.
"Installed" isn't a concept that always makes sense. On Windows, there's no system install. Even on other platforms, projects might download a hermetic toolchain to a random subdirectory.
|
We started seeing a build failure on our Mac toolchain builders after this patch: |
This doesn't feel right, I would expect it to say this: But it's not. It's linking to a full path. But the CMake changes responsible for this look the same as |
|
Looking closer, I just noticed this line is unchanged. I can make a patch for this. |
Summary: Goes in hand with #97130, split out to figure out CI fails. Should just build whatever subprojects utilize the `RISCVMCTargetDesc.h` header and it should build & test just like normal. Co-authored-by: pca006132 <[email protected]> Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251736
Summary: Continue from #87196 as author did not have much time, I have taken over working on this PR. We would like to have this so it'll be easier to package for Nix. Can be tested by copying cmake, bolt, third-party, and llvm directories out into their own directory with this PR applied and then build bolt. --------- Co-authored-by: pca006132 <[email protected]> Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250600
CC @gulfemsavrun Fixes a line which wasn't changed in #97130
Continue from #87196 as author did not have much time, I have taken over working on this PR. We would like to have this so it'll be easier to package for Nix.
Can be tested by copying cmake, bolt, third-party, and llvm directories out into their own directory with this PR applied and then build bolt.