Page MenuHomePhabricator

Linking NVPTX backend for Polly-ACC
ClosedPublic

Authored by singam-sanjay on Apr 13 2017, 12:53 AM.

Details

Summary

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.

Diff Detail

Repository
rL LLVM

Event Timeline

singam-sanjay created this revision.Apr 13 2017, 12:53 AM

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.

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.

I believe checking for the existence of NVPTX backend is not required, LLVM_LINK_COMPONENTS will already complain if it does not exist.

Alright. Do you recommend adding a warning message in <polly_src>/lib/CMakeLists.txt in case the libraries aren't found ?

I believe checking for the existence of NVPTX backend is not required, LLVM_LINK_COMPONENTS will already complain if it does not exist.

How can LLVM_LINK_COMPONENETS complain about missing NVPTX during the build is configured, if ${LLVM_TARGETS_TO_BUILD} were to only contain X86 ?

All targets are linked into bugpoint.

Wow, this still looks very complicated. I think we just want something like:

 set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
   Analysis
   BitWriter
   CodeGen
   Core

Would this work?

Best,
Tobias

@grosser @Meinersbur

Only target_link_libraries(bugpoint LLVMNVPTXCodeGen LLVMNVPTXInfo ...) worked.

 set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
   Analysis
   BitWriter
   CodeGen
   Core

Would this work?

That didn't work actually. None of the following combinations worked,

  1. set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} ... )
  2. set(LLVM_LINK_COMPONENTS ... ${LLVM_TARGETS_TO_BUILD} )
  3. set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} ... ${LLVM_TARGETS_TO_BUILD} )
  4. LIST(APPEND LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} )
  5. set(LLVM_LINK_COMPONENTS NVPTX ... )
  6. set(LLVM_LINK_COMPONENTS ... NVPTX )
  7. 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.

Latest working patch

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

singam-sanjay marked an inline comment as done.Apr 18 2017, 12:02 PM

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.

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:

  1. Add the target_link_libraries of the libs returned by llvm_map_components_to_libnames(${LLVM_TARGETS_TO_BUILD} ... for Polly.
  2. 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).
  3. Have bugpoint depend on the targets, like by having a call to LLVMInitializeAllTargets that is never executed.

That is, Polly is resolved last

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" ?

  1. 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.

  1. 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..." ?

  1. 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 ?

That is, Polly is resolved last

Aren't PollyPPCG and PollyISL are resolved at last ?

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?

  1. 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.

  1. 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.

  1. 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).

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)

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.

  1. 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.

  1. 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.

  1. 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.

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)

I'm sorry. I meant that the LLVMInitializeNVPTX functions are called in Support/RegisterPasses.cpp.

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.

  1. 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.

  1. 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.

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.

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)

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);

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 ?

I'll comment on code lines as soon the diff is updated.

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 ?

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...

Patch for Polly:

diff --git 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")

Will this work even when Polly is built out of tree ?

+ endif ()

endif (GPU_CODEGEN)

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.

Adapt to D32442

bugpoint requires to link to NVPTX for PollACC only when Polly isn't linked into it.

singam-sanjay added inline comments.Apr 27 2017, 12:36 PM
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.

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).

singam-sanjay added inline comments.Apr 28 2017, 3:29 AM
tools/bugpoint/CMakeLists.txt
19 ↗(On Diff #96980)

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).

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.

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.

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?

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 ?

  • Added header file required by bugpoint for InitializeAllTarget*()
  • Removed comment in LLVMBuild.txt, which is causing error during build config.

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,

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 ?

Yes. You can add others as well. The "worst" thing they could do is removing themselves from the reviewers list again.

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.

@Meinersbur did you get a chance to try the patch in the comment above ?

As long as D31859 is not committed (I though it would be necessary for Julia), it works even without this path.

tools/bugpoint/bugpoint.cpp
142 ↗(On Diff #97183)

This #ifndef can be removed as well.

singam-sanjay added inline comments.May 10 2017, 5:22 AM
tools/bugpoint/bugpoint.cpp
142 ↗(On Diff #97183)

Why would you want to unconditionally link the back-ends into bug-point ?

Meinersbur added inline comments.May 10 2017, 7:15 AM
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:

  • LINK_POLLY_INTO_TOOLS is defined, that is, bugpoint depends on Polly which depends on NVPTX, hence get linked in. The code here would have no effect.
  • LINK_POLLY_INTO_TOOLS is not defined, that is this code its enabled and causes the NVPTX to be linked in.

That is, it does not matter for Polly whether the code here is conditional or not.

Unconditionally linking $LLVM_TARGETS_TO_BUILD in agreement to https://reviews.llvm.org/D32003#750923

singam-sanjay edited the summary of this revision. (Show Details)
grosser accepted this revision.Jun 23 2017, 11:04 PM

This looks good to me now.

This revision is now accepted and ready to land.Jun 23 2017, 11:04 PM
This revision was automatically updated to reflect the committed changes.