This patch links LLVM back-ends to support tools, dependent on the back-ends, that link into bugpoint. The back-ends are already available in opt and clang.
For e.g. In D31859, Polly requires the NVPTX back-end.
Details
Details
- Reviewers
hfinkel bogner chandlerc grosser Meinersbur - Commits
- rGa269b81a9ff3: Ensure backends available in 'opt' are also available in 'bugpoint'
rG4bba4ba5bae3: Ensure backends available in 'opt' are also available in 'bugpoint'
rL306208: Ensure backends available in 'opt' are also available in 'bugpoint'
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
My expectation was to only add ${LLVM_TARGETS_TO_BUILD} to LLVM_LINK_COMPONENTS, like opt does. However, indeed only the NVPTX target is required.
I believe checking for the existence of NVPTX backend is not required, LLVM_LINK_COMPONENTS will already complain if it does not exist.
| tools/bugpoint/CMakeLists.txt | ||
|---|---|---|
| 58–61 ↗ | (On Diff #95083) | Consider removing commented code entirely. |
Comment Actions
I think for consistency we should add ${LLVM_TARGETS_TO_BUILD} to LLVM_LINK_COMPONENTS. This should not be about Polly. For modules in general the same libraries should be available in opt and bugpoint.
Comment Actions
Alright. Do you recommend adding a warning message in <polly_src>/lib/CMakeLists.txt in case the libraries aren't found ?
Comment Actions
How can LLVM_LINK_COMPONENETS complain about missing NVPTX during the build is configured, if ${LLVM_TARGETS_TO_BUILD} were to only contain X86 ?
Comment Actions
Wow, this still looks very complicated. I think we just want something like:
set(LLVM_LINK_COMPONENTS
+ ${LLVM_TARGETS_TO_BUILD}
Analysis
BitWriter
CodeGen
CoreWould this work?
Best,
Tobias
Comment Actions
Only target_link_libraries(bugpoint LLVMNVPTXCodeGen LLVMNVPTXInfo ...) worked.
That didn't work actually. None of the following combinations worked,
- set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} ... )
- set(LLVM_LINK_COMPONENTS ... ${LLVM_TARGETS_TO_BUILD} )
- set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} ... ${LLVM_TARGETS_TO_BUILD} )
- LIST(APPEND LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} )
- set(LLVM_LINK_COMPONENTS NVPTX ... )
- set(LLVM_LINK_COMPONENTS ... NVPTX )
- set(LLVM_LINK_COMPONENTS ... Linker NVPTX ObjCARCOpts ... ) #Alphabetical order
The backend libraries were never included in the command whenever NVPTX or ${LLVM_TARGETS_TO_BUILD} was added to the end, but were included when added to the front and middle, although the command failed. opt, whose build was configured in the same way as bugpoint by marking it as an LLVM TOOL using add_llvm_tool in its CMakeLists.txt which was picked up by add_llvm_implicit_projects(), built successfully irrespective of the position of ${LLVM_TARGETS_TO_BUILD} in LLVM_LINK_COMPONENTS.
It was interesting to find some of the libraries, including the target libraries, repeating in the command and those that weren't a part of LLVM_LINK_COMPONENTS.
[...] lib/libLLVMX86CodeGen.a lib/libLLVMX86AsmPrinter.a lib/libLLVMX86AsmParser.a lib/libLLVMX86Desc.a lib/libLLVMX86Info.a lib/libLLVMX86Disassembler.a lib/libLLVMX86Info.a lib/libLLVMX86Utils.a lib/libLLVMNVPTXCodeGen.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMNVPTXDesc.a lib/libLLVMNVPTXInfo.a lib/libLLVMNVPTXInfo.a lib/libLLVMAnalysis.a lib/libLLVMBitWriter.a lib/libLLVMCodeGen.a lib/libLLVMCore.a lib/libLLVMipo.a lib/libLLVMIRReader.a lib/libLLVMInstCombine.a lib/libLLVMInstrumentation.a lib/libLLVMLinker.a lib/libLLVMObjCARCOpts.a lib/libLLVMScalarOpts.a lib/libLLVMSupport.a lib/libLLVMTarget.a lib/libLLVMTransformUtils.a lib/libLLVMVectorize.a -lpthread lib/libPolly.a lib/libLLVMTarget.a lib/libLLVMGlobalISel.a lib/libLLVMX86AsmPrinter.a lib/libLLVMX86Utils.a lib/libLLVMMCDisassembler.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMAsmParser.a lib/libLLVMAsmPrinter.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMSelectionDAG.a lib/libLLVMCodeGen.a lib/libLLVMBitWriter.a lib/libLLVMScalarOpts.a lib/libLLVMInstCombine.a lib/libLLVMTarget.a lib/libLLVMTransformUtils.a lib/libLLVMAnalysis.a lib/libLLVMObject.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMBitReader.a lib/libLLVMProfileData.a lib/libLLVMCore.a lib/libLLVMSupport.a -lrt -ldl -ltinfo -lpthread -lz -lm lib/libLLVMDemangle.a lib/libPollyPPCG.a lib/libPollyISL.a -Wl,-rpath,"\$ORIGIN/../lib"
I assumed the extra libraries were dependences and the repetitions and reordering had to do something with correct order to link all the libraries.
The only CMake function that rearranges libraries and dependences is expand_topologically, which is never called during build configuration.bugpoint is added as an LLVM executable through add_llvm_executable(bugpoint ...) which calls llvm_config on LLVM_LINK_COMPONENTS. The libraries associated with each COMPONENT are enumerated by llvm_map_components_to_libnamesand set to link into the executable at explicit_llvm_config. Here's the call stack,
Call Stack (most recent call first): cmake/modules/LLVM-Config.cmake:100 (llvm_map_components_to_libnames) cmake/modules/LLVM-Config.cmake:93 (explicit_llvm_config) cmake/modules/AddLLVM.cmake:713 (llvm_config) cmake/modules/AddLLVM.cmake:814 (add_llvm_executable) tools/bugpoint/CMakeLists.txt:22 (add_llvm_tool)
I'm not sure what's causing the linking to fail.
I'm now looking at <cmake_src>/Source/cmTargetLinkLibrariesCommand.cxx for clues.
Please share your thoughts and suggestions.
Comment Actions
The following patch works for me:
diff --git a/tools/bugpoint/CMakeLists.txt b/tools/bugpoint/CMakeLists.txt index 7598657..8975e67 100644 --- a/tools/bugpoint/CMakeLists.txt +++ b/tools/bugpoint/CMakeLists.txt @@ -1,4 +1,5 @@ set(LLVM_LINK_COMPONENTS + ${LLVM_TARGETS_TO_BUILD} Analysis BitWriter CodeGen diff --git a/tools/bugpoint/LLVMBuild.txt b/tools/bugpoint/LLVMBuild.txt index 37a6058..68ecb8c 100644 --- a/tools/bugpoint/LLVMBuild.txt +++ b/tools/bugpoint/LLVMBuild.txt @@ -30,3 +30,4 @@ required_libraries = Linker ObjCARC Scalar + all-targets
Which error do you get exactly? Which setting of BUILD_SHARED_LIBS are you using? Can you post the error message?
Best,
Tobias
Comment Actions
For the following configuration command,
cmake -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD="X86;NVPTX" -DPOLLY_ENABLE_GPGPU_CODEGEN=ON ../llvm_src/ -G Ninja
I get the following error,
FAILED: : && /usr/bin/c++ -fPIC -fvisibility-inlines-hidden -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,-allow-shlib-undefined -Wl,--export-dynamic -Wl,-O3 -Wl,--gc-sections tools/bugpoint/CMakeFiles/bugpoint.dir/BugDriver.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/CrashDebugger.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/ExecutionDriver.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/ExtractFunction.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/FindBugs.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/Miscompilation.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/OptimizerDriver.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/ToolRunner.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/bugpoint.cpp.o -o bin/bugpoint lib/libLLVMX86CodeGen.a lib/libLLVMX86AsmPrinter.a lib/libLLVMX86AsmParser.a lib/libLLVMX86Desc.a lib/libLLVMX86Info.a lib/libLLVMX86Disassembler.a lib/libLLVMX86Info.a lib/libLLVMX86Utils.a lib/libLLVMNVPTXCodeGen.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMNVPTXDesc.a lib/libLLVMNVPTXInfo.a lib/libLLVMNVPTXInfo.a lib/libLLVMAnalysis.a lib/libLLVMBitWriter.a lib/libLLVMCodeGen.a lib/libLLVMCore.a lib/libLLVMipo.a lib/libLLVMIRReader.a lib/libLLVMInstCombine.a lib/libLLVMInstrumentation.a lib/libLLVMLinker.a lib/libLLVMObjCARCOpts.a lib/libLLVMScalarOpts.a lib/libLLVMSupport.a lib/libLLVMTarget.a lib/libLLVMTransformUtils.a lib/libLLVMVectorize.a -lpthread lib/libPolly.a lib/libLLVMTarget.a lib/libLLVMGlobalISel.a lib/libLLVMX86AsmPrinter.a lib/libLLVMX86Utils.a lib/libLLVMMCDisassembler.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMAsmParser.a lib/libLLVMAsmPrinter.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMSelectionDAG.a lib/libLLVMCodeGen.a lib/libLLVMBitWriter.a lib/libLLVMScalarOpts.a lib/libLLVMInstCombine.a lib/libLLVMTarget.a lib/libLLVMTransformUtils.a lib/libLLVMAnalysis.a lib/libLLVMObject.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMBitReader.a lib/libLLVMProfileData.a lib/libLLVMCore.a lib/libLLVMSupport.a -lrt -ldl -ltinfo -lpthread -lz -lm lib/libLLVMDemangle.a lib/libPollyPPCG.a lib/libPollyISL.a -Wl,-rpath,"\$ORIGIN/../lib" && : lib/libPolly.a(RegisterPasses.cpp.o):RegisterPasses.cpp:function polly::initializePollyPasses(llvm::PassRegistry&): error: undefined reference to 'LLVMInitializeNVPTXTarget' lib/libPolly.a(RegisterPasses.cpp.o):RegisterPasses.cpp:function polly::initializePollyPasses(llvm::PassRegistry&): error: undefined reference to 'LLVMInitializeNVPTXTargetInfo' lib/libPolly.a(RegisterPasses.cpp.o):RegisterPasses.cpp:function polly::initializePollyPasses(llvm::PassRegistry&): error: undefined reference to 'LLVMInitializeNVPTXTargetMC' lib/libPolly.a(RegisterPasses.cpp.o):RegisterPasses.cpp:function polly::initializePollyPasses(llvm::PassRegistry&): error: undefined reference to 'LLVMInitializeNVPTXAsmPrinter' collect2: error: ld returned 1 exit status ninja: build stopped: subcommand failed.
Comment Actions
I confirm @singam-sanjay linking error. It's a library resolution order problem. The linking line that comes out is
c++ ... lib/libLLVMNVPTXCodeGen.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMNVPTXDesc.a lib/libLLVMNVPTXInf o.a lib/libLLVMNVPTXInfo.a ... lib/libPollyPPCG.a lib/libPollyISL.a
That is, Polly is resolved last, it adds new unresolved symbols LLVMInitializeNVPTXTarget etc. which are not resolved anymore because the NVPTX archives they are defined in have already been processed. It works for opt because it pulled in these symbols before because another component requires them.
I can think of the following ways to resolve this:
- Add the target_link_libraries of the libs returned by llvm_map_components_to_libnames(${LLVM_TARGETS_TO_BUILD} ... for Polly.
- Change to library order (LLVM_LINK_COMPONENTS) such that Polly is linked before the target libraries. Note that adding Polly to the list does not work because component's libraries are prefixed with LLVM. LLVMPolly is the loadable module (I think the LLVMPolly and Polly targets have they names reversed).
- Have bugpoint depend on the targets, like by having a call to LLVMInitializeAllTargets that is never executed.
Comment Actions
Aren't PollyPPCG and PollyISL are resolved at last ? The LLVMInitializeNVPTX functions are defined in Support/RegisterPasses.cpp which are included only in libPolly.a .
, it adds new unresolved symbols LLVMInitializeNVPTXTarget etc. which are not resolved anymore because the NVPTX archives they are defined in have already been processed. It works for opt because it pulled in these symbols before because another component requires them.
Did you mean "dependences resolved" by "processed" ?
- Add the target_link_libraries of the libs returned by llvm_map_components_to_libnames(${LLVM_TARGETS_TO_BUILD} ... for Polly.
This is what the latest revision of the patch does.
- Change to library order (LLVM_LINK_COMPONENTS) such that Polly is linked before the target libraries. Note that adding Polly to the list does not work because component's libraries are prefixed with LLVM. LLVMPolly is the loadable module (I think the LLVMPolly and Polly targets have they names reversed).
Could you elucidate the last sentence, "LLVMPolly is the loadable module..." ?
- Have bugpoint depend on the targets, like by having a call to LLVMInitializeAllTargets that is never executed.
Something like what ForcePassLinking, in LinkAllPasses.h, does ?
Comment Actions
Yes, I mean these are the ones I meant with "Polly". libPolly.a is actually somewhere before, but the only thing that matters is that it is linked only after libLLVMNVPTXInfo.a.
The LLVMInitializeNVPTX functions are defined in Support/RegisterPasses.cpp which are included only in libPolly.a .
No. For instance, LLVMInitializeNVPTXTargetInfo is defined in libLLVMNVPTXInfo.a (NVPTXTargetInfo.cpp)
, it adds new unresolved symbols LLVMInitializeNVPTXTarget etc. which are not resolved anymore because the NVPTX archives they are defined in have already been processed. It works for opt because it pulled in these symbols before because another component requires them.
Did you mean "dependences resolved" by "processed" ?
Resolving symbols is a form of processing them, no?
- Add the target_link_libraries of the libs returned by llvm_map_components_to_libnames(${LLVM_TARGETS_TO_BUILD} ... for Polly.
This is what the latest revision of the patch does.
But the libraries are in the wrong order.
www.lurklurk.org/linkers/linkers.html#staticlibs might be a helpful read.
- Change to library order (LLVM_LINK_COMPONENTS) such that Polly is linked before the target libraries. Note that adding Polly to the list does not work because component's libraries are prefixed with LLVM. LLVMPolly is the loadable module (I think the LLVMPolly and Polly targets have they names reversed).
Could you elucidate the last sentence, "LLVMPolly is the loadable module..." ?
From lib/CMakeLists.txt:
add_polly_loadable_module(LLVMPolly Polly.cpp )
LLVM complaints when putting it into LLVM_LINK_COMPONENTS.
- Have bugpoint depend on the targets, like by having a call to LLVMInitializeAllTargets that is never executed.
Something like what ForcePassLinking, in LinkAllPasses.h, does ?
Kind of, but InitializeAllTargets() is currently called by opt and clang themselves (but not by bugpoint, we don't actually need to call it, just reference it so the linker has to resolve the symbol).
Comment Actions
I'm sorry. I meant that the LLVMInitializeNVPTX functions are called in Support/RegisterPasses.cpp.
Resolving symbols is a form of processing them, no?
Yes, it is.
- Add the target_link_libraries of the libs returned by llvm_map_components_to_libnames(${LLVM_TARGETS_TO_BUILD} ... for Polly.
This is what the latest revision of the patch does.
But the libraries are in the wrong order.
This https://gist.github.com/singam-sanjay/b62ad50b048b1a49d74454bb6bc0a929 is the command used to compile bugpoint (target_link_libraries add the NVPTX libraries to bugpoint), which lists the NVPTX files after libPolly.a, somehow.
- Change to library order (LLVM_LINK_COMPONENTS) such that Polly is linked before the target libraries. Note that adding Polly to the list does not work because component's libraries are prefixed with LLVM. LLVMPolly is the loadable module (I think the LLVMPolly and Polly targets have they names reversed).
Could you elucidate the last sentence, "LLVMPolly is the loadable module..." ?
From lib/CMakeLists.txt:
add_polly_loadable_module(LLVMPolly Polly.cpp )LLVM complaints when putting it into LLVM_LINK_COMPONENTS.
Alright.
- Have bugpoint depend on the targets, like by having a call to LLVMInitializeAllTargets that is never executed.
Something like what ForcePassLinking, in LinkAllPasses.h, does ?
Kind of, but InitializeAllTargets() is currently called by opt and clang themselves (but not by bugpoint, we don't actually need to call it, just reference it so the linker has to resolve the symbol).
Ohk. Would it be advisable to fence it by something like "#ifdef POLLY_GPU_CODEGEN" ? We might have to change the GPU_CODEGEN macro used in Polly's source for uniformity.
Comment Actions
That's too in the library order because the NVPTX symbols have already been resolved and won't be resolved again as explained in http://www.lurklurk.org/linkers/linkers.html#staticlibs.
- Add the target_link_libraries of the libs returned by llvm_map_components_to_libnames(${LLVM_TARGETS_TO_BUILD} ... for Polly.
This is what the latest revision of the patch does.
But the libraries are in the wrong order.
This https://gist.github.com/singam-sanjay/b62ad50b048b1a49d74454bb6bc0a929 is the command used to compile bugpoint (target_link_libraries add the NVPTX libraries to bugpoint), which lists the NVPTX files after libPolly.a, somehow.
Mine is:
/usr/bin/c++ -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -O3 -Wl,-allow-shlib-undefined -Wl,--export-dynamic -Wl,-rpath-link,/root/build/llvm/release/./lib -Wl,-O3 tools/bugpoint/CMakeFiles/bugpoint.dir/BugDriver.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/CrashDebugger.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/ExecutionDriver.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/ExtractFunction.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/FindBugs.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/Miscompilation.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/OptimizerDriver.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/ToolRunner.cpp.o tools/bugpoint/CMakeFiles/bugpoint.dir/bugpoint.cpp.o -o bin/bugpoint lib/libLLVMX86CodeGen.a lib/libLLVMX86AsmPrinter.a lib/libLLVMX86AsmParser.a lib/libLLVMX86Desc.a lib/libLLVMX86Info.a lib/libLLVMX86Disassembler.a lib/libLLVMX86Info.a lib/libLLVMX86Utils.a lib/libLLVMNVPTXCodeGen.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMNVPTXDesc.a lib/libLLVMNVPTXInfo.a lib/libLLVMNVPTXInfo.a lib/libLLVMAnalysis.a lib/libLLVMBitWriter.a lib/libLLVMCodeGen.a lib/libLLVMCore.a lib/libLLVMCoroutines.a lib/libLLVMipo.a lib/libLLVMIRReader.a lib/libLLVMInstCombine.a lib/libLLVMInstrumentation.a lib/libLLVMMC.a lib/libLLVMObjCARCOpts.a lib/libLLVMScalarOpts.a lib/libLLVMSupport.a lib/libLLVMTarget.a lib/libLLVMTransformUtils.a lib/libLLVMVectorize.a lib/libLLVMPasses.a -lpthread lib/libPolly.a lib/libLLVMGlobalISel.a lib/libLLVMX86AsmPrinter.a lib/libLLVMX86Utils.a lib/libLLVMMCDisassembler.a lib/libLLVMNVPTXAsmPrinter.a lib/libLLVMAsmPrinter.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMSelectionDAG.a lib/libLLVMCodeGen.a lib/libLLVMTarget.a lib/libLLVMipo.a lib/libLLVMBitWriter.a lib/libLLVMIRReader.a lib/libLLVMAsmParser.a lib/libLLVMLinker.a lib/libLLVMInstrumentation.a lib/libLLVMScalarOpts.a lib/libLLVMInstCombine.a lib/libLLVMVectorize.a lib/libLLVMTransformUtils.a lib/libLLVMAnalysis.a lib/libLLVMObject.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMBitReader.a lib/libLLVMProfileData.a lib/libLLVMCore.a lib/libLLVMSupport.a -lrt -ldl -ltinfo -lpthread -lz -lm lib/libLLVMDemangle.a lib/libPollyPPCG.a lib/libPollyISL.a -Wl,-rpath,"\$ORIGIN/../lib" -v
There might be some undefinedness as cmake just needs to find a topological ordering of libraries that satisfies all target_link_libraries dependencies.
- Have bugpoint depend on the targets, like by having a call to LLVMInitializeAllTargets that is never executed.
Something like what ForcePassLinking, in LinkAllPasses.h, does ?
Kind of, but InitializeAllTargets() is currently called by opt and clang themselves (but not by bugpoint, we don't actually need to call it, just reference it so the linker has to resolve the symbol).
Ohk. Would it be advisable to fence it by something like "#ifdef POLLY_GPU_CODEGEN" ? We might have to change the GPU_CODEGEN macro used in Polly's source for uniformity.
Sounds like a solution.
However, this is merely a workaround. Ideally, cmake should put the libraries into the right order in the first place. That would be done using target_link_libraries or LLVM_LINK_COMPONENTS.
Comment Actions
That is, the very first version you submitted here might be closer to what we want. The line
target_link_libraries(Polly
${NVLIBS}
)would ensure the correct order. The check whether the NVPTX is even available can be done by resolving the NVPTX libraries:
llvm_map_components_to_libnames(NVLIBS NVPTX)
which shows an error like this one of NVPTX is not available:
CMake Error at cmake/modules/LLVM-Config.cmake:259 (message): Library 'NVPTX' is a direct reference to a target library for an omitted target. Call Stack (most recent call first): cmake/modules/LLVM-Config.cmake:100 (llvm_map_components_to_libnames)
Comment Actions
This works for me:
Patch for bugpoint:
diff --git a/tools/bugpoint/CMakeLists.txt b/tools/bugpoint/CMakeLists.txt index 7598657427e..a48661a731f 100644 --- a/tools/bugpoint/CMakeLists.txt +++ b/tools/bugpoint/CMakeLists.txt @@ -39,4 +39,9 @@ if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) target_link_libraries(bugpoint Polly) # Ensure LLVMTarget can resolve dependences in Polly. target_link_libraries(bugpoint LLVMTarget) + + if (POLLY_ENABLE_GPGPU_CODEGEN) + llvm_map_components_to_libnames(NVLIBS NVPTX) + target_link_libraries(bugpoint ${NVLIBS}) + endif () endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
Patch for Polly:
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 8842ca91..9183c6bc 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -68,6 +68,9 @@ add_polly_library(Polly if (GPU_CODEGEN) target_link_libraries(Polly PollyPPCG) + if (DEFINED LLVM_MAIN_SRC_DIR AND NOT TARGET LLVMNVPTXInfo) + message(FATAL_ERROR "Polly-ACC requires NVPTX target to work") + endif () endif (GPU_CODEGEN) target_link_libraries(Polly ${ISL_TARGET}) @@ -90,6 +93,13 @@ if (BUILD_SHARED_LIBS) LLVMTarget LLVMVectorize ) + if (GPU_CODEGEN) + target_link_libraries(Polly + LLVMNVPTXCodeGen + LLVMNVPTXInfo + LLVMNVPTXDesc + LLVMNVPTXAsmPrinter) + endif () link_directories( ${LLVM_LIBRARY_DIR} ) diff --git a/lib/Support/RegisterPasses.cpp b/lib/Support/RegisterPasses.cpp index 9c8eac03..763bc8c7 100644 --- a/lib/Support/RegisterPasses.cpp +++ b/lib/Support/RegisterPasses.cpp @@ -35,6 +35,7 @@ #include "polly/Support/DumpModulePass.h" #include "llvm/Analysis/CFGPrinter.h" #include "llvm/IR/LegacyPassManager.h" +#include "llvm/Support/TargetSelect.h" #include "llvm/Transforms/IPO.h" #include "llvm/Transforms/IPO/PassManagerBuilder.h" #include "llvm/Transforms/Scalar.h" @@ -205,6 +206,10 @@ void initializePollyPasses(PassRegistry &Registry) { #ifdef GPU_CODEGEN initializePPCGCodeGenerationPass(Registry); + LLVMInitializeNVPTXTarget(); + LLVMInitializeNVPTXTargetInfo(); + LLVMInitializeNVPTXTargetMC(); + LLVMInitializeNVPTXAsmPrinter(); #endif initializeCodePreparationPass(Registry); initializeDeadCodeElimPass(Registry);
Comment Actions
I'm trying this out,
Patch of LLVM
diff --git a/tools/bugpoint/CMakeLists.txt b/tools/bugpoint/CMakeLists.txt index 7598657..e1d99cf 100644 --- a/tools/bugpoint/CMakeLists.txt +++ b/tools/bugpoint/CMakeLists.txt @@ -1,4 +1,5 @@ set(LLVM_LINK_COMPONENTS + ${LLVM_TARGETS_TO_BUILD} Analysis BitWriter CodeGen diff --git a/tools/bugpoint/bugpoint.cpp b/tools/bugpoint/bugpoint.cpp index 85c1ddd..0a5a851 100644 --- a/tools/bugpoint/bugpoint.cpp +++ b/tools/bugpoint/bugpoint.cpp @@ -26,6 +26,7 @@ #include "llvm/Support/PrettyStackTrace.h" #include "llvm/Support/Process.h" #include "llvm/Support/Signals.h" +#include "llvm/Support/TargetSelect.h" #include "llvm/Support/Valgrind.h" #include "llvm/Transforms/IPO/AlwaysInliner.h" #include "llvm/Transforms/IPO/PassManagerBuilder.h" @@ -138,6 +139,15 @@ int main(int argc, char **argv) { polly::initializePollyPasses(Registry); #endif +//#ifdef GPU_CODEGEN + if (std::getenv("bar") == (char*) -1) { + InitializeAllTargets(); + InitializeAllTargetMCs(); + InitializeAllAsmPrinters(); + } +//#endif + + cl::ParseCommandLineOptions(argc, argv, "LLVM automatic testcase reducer. See\nhttp://" "llvm.org/cmds/bugpoint.html"
Patch for Polly:
diff --git a/lib/Support/RegisterPasses.cpp b/lib/Support/RegisterPasses.cpp index 9c8eac0..763bc8c 100644 --- a/lib/Support/RegisterPasses.cpp +++ b/lib/Support/RegisterPasses.cpp @@ -35,6 +35,7 @@ #include "polly/Support/DumpModulePass.h" #include "llvm/Analysis/CFGPrinter.h" #include "llvm/IR/LegacyPassManager.h" +#include "llvm/Support/TargetSelect.h" #include "llvm/Transforms/IPO.h" #include "llvm/Transforms/IPO/PassManagerBuilder.h" #include "llvm/Transforms/Scalar.h" @@ -205,6 +206,10 @@ void initializePollyPasses(PassRegistry &Registry) { #ifdef GPU_CODEGEN initializePPCGCodeGenerationPass(Registry); + LLVMInitializeNVPTXTarget(); + LLVMInitializeNVPTXTargetInfo(); + LLVMInitializeNVPTXTargetMC(); + LLVMInitializeNVPTXAsmPrinter(); #endif initializeCodePreparationPass(Registry); initializeDeadCodeElimPass(Registry); diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index 18948ce..5904ba0 100644 --- a/unittests/CMakeLists.txt +++ b/unittests/CMakeLists.txt @@ -17,6 +17,15 @@ function(add_polly_unittest test_name) set_property(TARGET ${test_name} PROPERTY FOLDER "Polly") endif() target_link_libraries(${test_name} Polly LLVMCore LLVMSupport LLVMDemangle LLVMipo) + if (GPU_CODEGEN) + target_link_libraries(${test_name} + LLVMNVPTXCodeGen + LLVMNVPTXInfo + LLVMNVPTXDesc + LLVMNVPTXAsmPrinter + ) + endif (GPU_CODEGEN) + endfunction() add_subdirectory(Isl)
I didn't want to link NVPTX libs to Polly since that's against LLVM's design principles (discussion during Polly call on 12/4/17). But, linking them to Polly does prevent the requirement to link them to Polly's dependents that don't already link the NVPTX libs e.g. DeLICM. Shall I go ahead with linking them to Polly ?
Comment Actions
I'll comment on code lines as soon the diff is updated.
You mean the principle that tools should not only work on a single target?
Yes, it would be nicer if we do not have special treatment for individual back-ends. In this case the configure process will/should fail anyway when WITH_POLLY and POLLY_ENABLE_GPGPU_CODEGEN are enabled, so there is no loss. Instead of NVPTX one could also insert ${LLVM_TARGETS_TO_BUILD}.
Your code above does some unecessary initialization. I'd find that itself accaptable because it is the same behavior as opt. But as a solution to our problem it feels like a hack in that we try to hide the true dependency between Polly and the NVPTX backend.
Another alternative already mentioned is to use weak symbols, so we don't get a link error just for bugpoint.
Now that I remember what the reason was we cannot use target_link_libraries(Polly LLVMNVPTXInfo ...) in Polly, I have a solution that I had in mind some time for it that unfortunately requires to rework Polly's build system. I'll will give it a try...
Comment Actions
Will this work even when Polly is built out of tree ?
+ endif ()
endif (GPU_CODEGEN)
Comment Actions
Latest working patch to include NVPTX libraries for Polly-ACC. Commented code is a proposed extension to the current patch, and will include or exclude it after discussion.
Comment Actions
Adapt to D32442
bugpoint requires to link to NVPTX for PollACC only when Polly isn't linked into it.
| tools/bugpoint/CMakeLists.txt | ||
|---|---|---|
| 42 ↗ | (On Diff #96980) | LINK_POLLY_INTO_TOOLS is set to ON only when both WITH_POLLY and LLVM_POLLY_LINK_INTO_TOOLS are ON in <llvm_src>/CMakeLists.txt |
| tools/bugpoint/bugpoint.cpp | ||
| 141 ↗ | (On Diff #96308) | A POLLY_GPU_CODEGEN macro can defined to add another fence to include the code ( and the libraries) only when PollyACC is being built. I'm guessing it should be #cmakedefined at include/llvm/Config/{config.h.cmake,llvm-config.h.cmake} and set to true at <llvm_src>/CMakeLists.txt for the "-DPOLLY_GPU_CODEGEN" flag to be added to the build command. |
Comment Actions
This patch is not needed after rL301558.
NVPTX will be pulled-in by
target_link_libraries(bugpoint Polly)
since Polly depends on the NVPTX* libraries.
There is still the case with LINK_POLLY_INTO_TOOLS=OFF when NVPTX is not in bugpoint, and also not in LLVMPolly.so. However, in this case this patch doesn't help either since it is conditional on LINK_POLLY_INTO_TOOLS (as should be, LLVMPolly.so is designed to be loaded into tools that have not been compiled with Polly).
| tools/bugpoint/CMakeLists.txt | ||
|---|---|---|
| 19 ↗ | (On Diff #96980) | NVPTX is added to LLVM_LINK_COMPONENTS when LINK_POLLY_INTO_TOOLS=OFF over here. To ensure that NVPTX is linked into the bugpoint binary, practically unreachable calls to InitalizeAllTargets*() are included in bugpoint.cpp when the LINK_POLLY_INTO_TOOLS macro isn't defined. So linking NVPTX into Polly is conditional on NOT LINK_POLLY_INTO_TOOLS. |
| tools/bugpoint/bugpoint.cpp | ||
| 141 ↗ | (On Diff #96980) | To force NVPTX to link into bugpoint when Polly isn't linked into bugpoint, calls to InitializeAllTargets*() functions are included in the source which are fenced by a condition that never evaluates to true, but cannot be evaluated by the compiler statically to optimise it away as dead code. |
Comment Actions
You are right, I missed that it is #ifndef LINK_POLLY_INTO_TOOLS and NOT LINK_POLLY_INTO_TOOLS.
If our goal is to have a uniform bugpoint in which LLVMPolly.so works, independent of whether Polly is in the source tree or not, then we don't to do it conditionally on LINK_POLLY_INTO_TOOLS at all. In both cases we want NVPTX to be linked into it, and it is doesn't matter whether or not target_link_libraries(bugpoint Polly) drags in a dependency into bugpoint. Don't you think?
Did you try whether bugpoint work with LLVMPolly.so? I just realized I never did.
In any case, we should get approval from someone from LLVM for making changes that not only concerns Polly. Consider adding some other reviewers as well.
Comment Actions
Yes, I agree that should be the case for uniformity. But, what if developers who aren't using Polly-ACC don't want NVPTX to be linked in for some reason ?
Did you try whether bugpoint work with LLVMPolly.so? I just realized I never did.
bugpoint crashed when LLVMPolly.so was loaded without linking NVPTX into bugpoint.
➜ llvm_build bin/bugpoint -load lib/LLVMPolly.so -help Error opening 'lib/LLVMPolly.so': lib/LLVMPolly.so: undefined symbol: LLVMInitializeNVPTXTarget -load request ignored. #0 0x0000000000cd41ba llvm::sys::PrintStackTrace(llvm::raw_ostream&) (bin/bugpoint+0xcd41ba) #1 0x0000000000cd240e llvm::sys::RunSignalHandlers() (bin/bugpoint+0xcd240e) #2 0x0000000000cd254a SignalHandler(int) (bin/bugpoint+0xcd254a) #3 0x00007f763cdb2330 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330) #4 0x0000000000c71b24 sortOpts(llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>&, llvm::SmallVectorImpl<std::pair<char const*, llvm::cl::Option*> >&, bool) (bin/bugpoint+0xc71b24) #5 0x0000000000c79813 (anonymous namespace)::HelpPrinter::operator=(bool) [clone .part.241] (bin/bugpoint+0xc79813) #6 0x0000000000c79f7f llvm::cl::opt<(anonymous namespace)::HelpPrinterWrapper, true, llvm::cl::parser<bool> >::handleOccurrence(unsigned int, llvm::StringRef, llvm::StringRef) (bin/bugpoint+0xc79f7f) #7 0x0000000000c742b1 llvm::cl::Option::addOccurrence(unsigned int, llvm::StringRef, llvm::StringRef, bool) (bin/bugpoint+0xc742b1) #8 0x0000000000c741d4 ProvideOption(llvm::cl::Option*, llvm::StringRef, llvm::StringRef, int, char const* const*, int&) (bin/bugpoint+0xc741d4) #9 0x0000000000c77f8e (anonymous namespace)::CommandLineParser::ParseCommandLineOptions(int, char const* const*, llvm::StringRef, llvm::raw_ostream*) (bin/bugpoint+0xc77f8e) #10 0x000000000056c89e main (bin/bugpoint+0x56c89e) #11 0x00007f763bb95f45 __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:321:0 #12 0x00000000005996a7 _start (bin/bugpoint+0x5996a7) Stack dump: 0. Program arguments: bin/bugpoint -load lib/LLVMPolly.so -help [1] 12006 segmentation fault (core dumped) bin/bugpoint -load lib/LLVMPolly.so -help
When NVPTX is linked, bin/bugpoint -load lib/LLVMPolly.so -help prints the help message along with the options for Polly.
In any case, we should get approval from someone from LLVM for making changes that not only concerns Polly. Consider adding some other reviewers as well.
Justin Bogner (@ bogner) has made the latest changes to bugpoint.cpp. Do you suggest adding him ?
Comment Actions
- Added header file required by bugpoint for InitializeAllTarget*()
- Removed comment in LLVMBuild.txt, which is causing error during build config.
Comment Actions
Here's a diff from the current patch that links NVPTX only when LINK_POLLY_INTO_TOOLS=OFF and POLLY_ENABLE_GPGPU_CODEGEN=ON, i.e. only when bugpoint needs it. The current patch links NVPTX even when POLLY_ENABLE_GPGPU_CODEGEN=OFF just because LINK_POLLY_INTO_TOOLS=OFF.
diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f5df77..913df66 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -451,6 +451,14 @@ else() set(LINK_POLLY_INTO_TOOLS OFF) endif() +if(POLLY_ENABLE_GPGPU_CODEGEN) + set(POLLY_GPU_CODEGEN ON) +else() + set(POLLY_GPU_CODEGEN OFF) +endif() + # Define an option controlling whether we should build for 32-bit on 64-bit # platforms, where supported. if( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32 ) diff --git a/include/llvm/Config/config.h.cmake b/include/llvm/Config/config.h.cmake index a64e208..2c9886b 100644 --- a/include/llvm/Config/config.h.cmake +++ b/include/llvm/Config/config.h.cmake @@ -334,6 +334,9 @@ /* Define if we link Polly to the tools */ #cmakedefine LINK_POLLY_INTO_TOOLS +/* Define if Polly can generate GPU code */ +#cmakedefine POLLY_GPU_CODEGEN + /* Target triple LLVM will generate code for by default */ /* Doesn't use `cmakedefine` because it is allowed to be empty. */ #define LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}" diff --git a/include/llvm/Config/llvm-config.h.cmake b/include/llvm/Config/llvm-config.h.cmake index 4b0c594..ed5bb2d 100644 --- a/include/llvm/Config/llvm-config.h.cmake +++ b/include/llvm/Config/llvm-config.h.cmake @@ -17,6 +17,9 @@ /* Define if we link Polly to the tools */ #cmakedefine LINK_POLLY_INTO_TOOLS +/* Define if Polly can generate GPU code */ +#cmakedefine POLLY_GPU_CODEGEN + /* Target triple LLVM will generate code for by default */ #cmakedefine LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}" diff --git a/tools/bugpoint/CMakeLists.txt b/tools/bugpoint/CMakeLists.txt index d33be24..1456205 100644 --- a/tools/bugpoint/CMakeLists.txt +++ b/tools/bugpoint/CMakeLists.txt @@ -16,9 +16,11 @@ set(LLVM_LINK_COMPONENTS Vectorize ) -if (NOT LINK_POLLY_INTO_TOOLS) +if (NOT LINK_POLLY_INTO_TOOLS AND POLLY_GPU_CODEGEN) list(APPEND LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD}) -endif (NOT LINK_POLLY_INTO_TOOLS) +endif (NOT LINK_POLLY_INTO_TOOLS AND POLLY_GPU_CODEGEN) + +message(STATUS ${LLVM_LINK_COMPONENTS}) # Support plugins. set(LLVM_NO_DEAD_STRIP 1) diff --git a/tools/bugpoint/bugpoint.cpp b/tools/bugpoint/bugpoint.cpp index d05044c..6833550 100644 --- a/tools/bugpoint/bugpoint.cpp +++ b/tools/bugpoint/bugpoint.cpp @@ -140,12 +140,16 @@ int main(int argc, char **argv) { #endif #ifndef LINK_POLLY_INTO_TOOLS +#ifdef POLLY_GPU_CODEGEN if (std::getenv("bar") == (char*) -1) { InitializeAllTargets(); InitializeAllTargetMCs(); InitializeAllAsmPrinters(); } #endif +#endif cl::ParseCommandLineOptions(argc, argv,
Comment Actions
Yes. You can add others as well. The "worst" thing they could do is removing themselves from the reviewers list again.
| tools/bugpoint/bugpoint.cpp | ||
|---|---|---|
| 142 ↗ | (On Diff #97183) | Why would you want to unconditionally link the back-ends into bug-point ? |
| tools/bugpoint/bugpoint.cpp | ||
|---|---|---|
| 142 ↗ | (On Diff #97183) | To get a uniform bugpoint build that has the backend symbols available. There might be other tools than Polly that generate heterogeneous code and require the backend's symbols to be contained in the host program. To try to be less specific to Polly. In case of Polly, there are only the cases:
That is, it does not matter for Polly whether the code here is conditional or not. |
Comment Actions
Unconditionally linking $LLVM_TARGETS_TO_BUILD in agreement to https://reviews.llvm.org/D32003#750923