The NVPTX backend is now initialised within Polly. A language front-end need not be modified to initialise the backend, just for Polly.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
With -DLLVM_TARGETS_TO_BUILD=X86 (i.e. without nvptx backend) I get this error:
FAILED: /usr/bin/c++ -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/polly/lib -I/root/src/llvm/tools/polly/lib -Itools/polly/include -I/root/src/llvm/tools/polly/lib/External -I/root/src/llvm/tools/polly/lib/External/pet/include -I/root/src/llvm/tools/polly/lib/External/JSON/include -I/root/src/llvm/tools/polly/lib/External/isl/include -Itools/polly/lib/External/isl/include -I/root/src/llvm/tools/polly/include -Iinclude -I/root/src/llvm/include -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 -fno-common -Woverloaded-virtual -Wno-long-long -Wall -W -Wno-unused-parameter -Wwrite-strings -fno-exceptions -fno-rtti -O3 -UNDEBUG -MMD -MT tools/polly/lib/CMakeFiles/Polly.dir/Support/RegisterPasses.cpp.o -MF tools/polly/lib/CMakeFiles/Polly.dir/Support/RegisterPasses.cpp.o.d -o tools/polly/lib/CMakeFiles/Polly.dir/Support/RegisterPasses.cpp.o -c /root/src/llvm/tools/polly/lib/Support/RegisterPasses.cpp
/root/src/llvm/tools/polly/lib/Support/RegisterPasses.cpp: In function ‘void polly::initializePollyPasses(llvm::PassRegistry&)’:
/root/src/llvm/tools/polly/lib/Support/RegisterPasses.cpp:210:31: error: ‘LLVMInitializeNVPTXTarget’ was not declared in this scope
LLVMInitializeNVPTXTarget();
^
/root/src/llvm/tools/polly/lib/Support/RegisterPasses.cpp:211:35: error: ‘LLVMInitializeNVPTXTargetInfo’ was not declared in this scope
LLVMInitializeNVPTXTargetInfo();
^
/root/src/llvm/tools/polly/lib/Support/RegisterPasses.cpp:212:33: error: ‘LLVMInitializeNVPTXTargetMC’ was not declared in this scope
LLVMInitializeNVPTXTargetMC();
^
/root/src/llvm/tools/polly/lib/Support/RegisterPasses.cpp:213:35: error: ‘LLVMInitializeNVPTXAsmPrinter’ was not declared in this scope
LLVMInitializeNVPTXAsmPrinter();
^Comment Actions
If there were calls to LLVMInitializeNVPTX*(), then GPU_CODEGEN was defined without building the NVPTX backend. Can you check if ENABLE_POLLY_GPU_CODEGEN set to ON with only X86 being built ?
Comment Actions
ENABLE_POLLY_GPU_CODEGEN=ON in my case. If that setting requires ENABLE_POLLY_GPU_CODEGEN=NVPTX, it should be checked in CMakeLists.txt, instead of failing randomly when compiling.
Note that we might add an OpenCL/SPIR backend to Polly-ACC in the future that does not require the NVPTX backend.
Comment Actions
The modifications to integrate the OpenCL backend would depend on whether we would let Polly choose between backends at runtime or compile time.
i.e. the difference between having "-polly-target=gpu_ocl or -polly-target=gpu_cuda" and just "-polly-target=gpu" with one of the GPU backends being built.
We could even try to figure the type of GPU installed and build/use the appropriate backend.
Comment Actions
Using -DPOLLY_ENABLE_GPGPU_CODEGEN=ON -DLLVM_POLLY_LINK_INTO_TOOLS=OFF I get (ninja check-polly):
opt: CommandLine Error: Option 'nvptx-emit-line-numbers' registered more than once!
Using -DPOLLY_ENABLE_GPGPU_CODEGEN=ON -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON or -DPOLLY_ENABLE_GPGPU_CODEGEN=ON -DBUILD_SHARED_LIBS=ON I get:
opt: Unknown command line argument '-polly-codegen-ppcg'. Try: 'opt -help' opt: Did you mean '-polly-codegen'?
Comment Actions
@grosser The build stops when the LLVMInitializeNVPTX*() functions aren't found. These are made available by linking the libraries, which cannot by done by including header files.
Answering your question here,
On Wed, 12 Apr 2017 at 00:36 Tobias Grosser wrote:
How do the header file requirements influence what we link into Polly?
What will break if we do not change the linking?
Best,
Tobias
On Tue, Apr 11, 2017, at 08:07 PM, Sanjay Srivallabh Singapuram wrote:
That's not working because the function definitions aren't available to
Polly through the headers.On Tue 11 Apr, 2017, 11:35 PM Tobias Grosser wrote:
I don't think we need to change anything with the linking. We should
just call the functions when needed.Best,
Tobias
Comment Actions
Could we maybe move the responsibility to initialize the NVPTX target to the host application? opt and clang do this already by calling LLVMInitializeAllTargets.
Comment Actions
What exactly is causing each of these issues ?
I guess here the problem is with the redeclaration of EmitLineNumbers. This isn't an issue when Polly is linked into tools because only one copy of LLVMNVPTX* libraries is linked into the program, either Polly's or the ones linked through the command line. Please correct me if I'm wrong.
only
Using -DPOLLY_ENABLE_GPGPU_CODEGEN=ON -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON or -DPOLLY_ENABLE_GPGPU_CODEGEN=ON -DBUILD_SHARED_LIBS=ON I get:opt: Unknown command line argument '-polly-codegen-ppcg'. Try: 'opt -help' opt: Did you mean '-polly-codegen'?
Even I faced this when I ran ninja check-polly. I couldn't understand why this was happening since -polly-codegen-gen wasn't declared as a cl::opt.
This patch was meant to ease Polly's integration into frontends by taking care of the backend initialization. E.g. Julia developers weren't comfortable with calling LLVMInitializeAllTargets (https://github.com/JuliaLang/julia/pull/21142/commits/3371d635da12444f81034ca02a668fe9cefd7388#r107699287). So, the onus to initialize the backend now falls on Polly. For Julia it could be done in a module dedicated for Polly, but the effort still has to be repeated for every frontend that integrates with Polly.
Comment Actions
I tried that first. But it required initializing all backends and hence linking all backends.
Comment Actions
For this edit in RegisterPasses.cpp,
#ifdef GPU_CODEGEN
if (Target == TARGET_GPU) {
initializePPCGCodeGenerationPass(Registry);
InitializeAllTargets();
/*LLVMInitializeNVPTXTarget();
LLVMInitializeNVPTXTargetInfo();
LLVMInitializeNVPTXTargetMC();
LLVMInitializeNVPTXAsmPrinter();*/
}
#endifI get the following error when I don't link the NVPTX libraries.
[280/280] Linking CXX executable bin/bugpoint 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/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/libLLVMBitWriter.a lib/libLLVMAsmParser.a lib/libLLVMInstCombine.a lib/libLLVMTransformUtils.a lib/libLLVMAnalysis.a lib/libLLVMObject.a lib/libLLVMBitReader.a lib/libLLVMMCParser.a lib/libLLVMMC.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 'LLVMInitializeX86TargetInfo' 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 'LLVMInitializeX86Target' lib/libPolly.a(RegisterPasses.cpp.o):RegisterPasses.cpp:function polly::initializePollyPasses(llvm::PassRegistry&): error: undefined reference to 'LLVMInitializeNVPTXTarget' collect2: error: ld returned 1 exit status ninja: build stopped: subcommand failed.
Will linking to LLVNNVPTX*.so instead of LLVMNVPTX*.a help ? We could build them just for Polly.
Comment Actions
By host-program I assume you mean the program that converts a language into LLVM-IR and orchestrates everything. So, what you're saying is it's up to the host-program to initialize and provide what Polly needs. Have I understood you correctly ?
I still think linking is not the problem here, the same library linked-in twice is resolved by the linker and loader.
The opt: CommandLine Error: Option 'nvptx-emit-line-numbers' registered more than once! error that you received here could be a link error because you were linking archives not shared libraries. I didn't receive this error during ninja check-polly when I built LLVM with -DPOLLY_ENABLE_GPGPU_CODEGEN=ON -DLLVM_POLLY_LINK_INTO_TOOLS=OFF -DBUILD_SHARED_LIBS=ON.
The problem is that opt and clang already call LLVMInitializeAllTargets which in turn initializes the NVPTX target, and we do it again.
I'm not sure is that's a problem. As a part of this discussion on the polly-dev mailing list, I checked if reinitializing the backends was a problem by calling InitializeAllTarget*() functions in a loop with 10 iterations. The only effect it had was delaying the start by few hundredths of a microsecond.
There seems to be no protection for double-initialization.
One way is to check if the NVPTX target triples are already available through TargetRegistry::lookupTarget. But, I'm not sure if the availability of the target triples means that LLVMInitialize*TargetMC and LLVMInitialize*AsmPrinter were called.
Comment Actions
I think Tobias is right and this is more of a linking problem. It starts with bugpoint to not depend on the llvm targets (It instead calls opt to do the main work, itself it only needs to know which passes are available). Polly-ACC seems to be the only pass that has a link-dependency against a specific backend.
I suggested to Sanjay to create a patch that links bugpoint against the backends, just like opt does. bugpoint is an LLVM-developer only tool so there shouldn't be a lot of resistance against that. If there is, we can use weak symbols so linking does not fail in bugpoint (and its never used in bugpoint).
Comment Actions
I've made these changes to <llvm_src>/tools/bugpoint/CMakeLists.txt
-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)
-endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
+function(chk_GPU_lib_for_Polly lib_name)
+ if (NOT TARGET ${lib_name})
+ message(FATAL_ERROR
+ "${lib_name} not being built, is required by Polly-ACC.")
+ endif (NOT TARGET ${lib_name})
+endfunction(chk_GPU_lib_for_Polly)
+
+if (WITH_POLLY)
+ if (LINK_POLLY_INTO_TOOLS)
+ target_link_libraries(bugpoint Polly)
+ # Ensure LLVMTarget can resolve dependences in Polly.
+ target_link_libraries(bugpoint LLVMTarget)
+ endif (LINK_POLLY_INTO_TOOLS)
+ if (POLLY_ENABLE_GPGPU_CODEGEN)
+ chk_GPU_lib_for_Polly(LLVMNVPTXCodeGen)
+ chk_GPU_lib_for_Polly(LLVMNVPTXInfo)
+ chk_GPU_lib_for_Polly(LLVMNVPTXDesc)
+ chk_GPU_lib_for_Polly(LLVMNVPTXAsmPrinter)
+ LIST(APPEND LLVM_LINK_COMPONENTS
+ NVPTX
+ #LLVMNVPTXCodeGen
+ #LLVMNVPTXInfo
+ #LLVMNVPTXDesc
+ #LLVMNVPTXAsmPrinter
+ )When POLLY_ENABLE_GPGPU_CODEGEN is enabled, NVPTX links to bugpoint irrespective of whether Polly is linked into bugpoint. Does this fall under LLVM's design principles ?
Comment Actions
Linking NVPTX into Polly causes problem when NVPTX is linked separately e.g. opt and clang already link all backends, including NVPTX. This removes the onus of linking NVPTX from Polly and assumes that tools that link to Polly also take care of linking NVPTX. D32003 is one such change to LLVM's bugpoint.
Comment Actions
Unconditionally initializing the NVPTX backend since the Target variable is still isn't assigned to the value passed as a command line option when initializePollyPasses is called in bugpoint.cpp.
Comment Actions
Using target_link_libraries( Polly INTERFACE ... ) to prevent linking twice and resolve all dependences within Polly.
Comment Actions
With -DLLVM_POLLY_LINK_INTO_TOOLS=OFF (everything else left as default) I get:
opt: CommandLine Error: Option 'nvptx-emit-line-numbers' registered more than once!
I don't think target_link_libraries(Polly INTERFACE ...) solves our problems.
| lib/CMakeLists.txt | ||
|---|---|---|
| 70 ↗ | (On Diff #96114) | With an out-of-LLVM source tree I get CMake Error at lib/CMakeLists.txt:70 (llvm_map_components_to_libnames): Unknown CMake command "llvm_map_components_to_libnames". |
Comment Actions
I have a patch that's working with -DLLVM_POLLY_LINK_INTO_TOOLS=OFF. I'm testing it right now with this https://gist.github.com/singam-sanjay/7b6907c9b31385239f62c2e24068c53f script in different build configurations.
| lib/CMakeLists.txt | ||
|---|---|---|
| 70 ↗ | (On Diff #96114) | Then, listing all the NVPTX libraries would be required to work in all cases. Would it work if Polly had its own copy of "llvm_map_components_to_libnames" ? |
Comment Actions
The patch I was talking about didn't pass.
I see that when LLVM_POLLY_LINK_INTO_TOOLS=OFF, opt uses Polly from LLVMPolly.so which pulls in the NVPTX libraries when libLLVMNVPTX*.so or libLLVM.so aren't available. The error occurs when both the executable (opt in this case) and LLVMPolly.so have a copy of the same library. Is there a way to selectively remove libraries from LLVMPolly.so after it's linked to libPolly.a and its dependences ?
Also, what are benefits of not linking Polly into tools ?
Comment Actions
No, there isn't. It is the reason why there is no target_link_libraries(Polly LLVMCore LLVMSupport ...).
Also, what are benefits of not linking Polly into tools ?
First, you may not want to get the additional complexity of having Polly in your clang only because you checked out its source. This is much less of an argument since LLVM_POLLY_LINK_INTO_TOOLS=ON became the default.
However, this is irrelevant as long as we have the LLVMPolly.so` loadable module (which allows using Polly in a precompiled clang) which still needs to work. Without the switch LLVM_POLLY_LINK_INTO_TOOLS, we cannot easily test whether it is still working.
| lib/CMakeLists.txt | ||
|---|---|---|
| 70 ↗ | (On Diff #96114) | One might use the include command to import that function, but we get into trouble with out-of-llvm-tree builds. I don't think it is worth the hassle. |
Comment Actions
I didn't get you.
However, this is irrelevant as long as we have the LLVMPolly.so` loadable module (which allows using Polly in a precompiled clang) which still needs to work. Without the switch LLVM_POLLY_LINK_INTO_TOOLS, we cannot easily test whether it is still working.
Is it required for a Windows build ?
Comment Actions
Hi Sanjay,
it seems this patch is not needed any more. You might want to close this review.
Comment Actions
Did you mean @grosser? I was under the impression that calling LLVMInitializeNVPTX* was necessary for Julia, so I am as well a bit surprised that it is not necessary anymore.
@grosser Did you notice that my patch to the Polly build system does not call LLVMInitializeNVPTX* yet. It only ensures that the NVPTX libraries are available, which is required for any LLVMInitializeNVPTX* to not result in a link error.
Comment Actions
I addressed this to you since @grosser had resigned and assumed that he wouldn't be getting updates.
I was under the impression that calling LLVMInitializeNVPTX* was necessary for Julia, so I am as well a bit surprised that it is not necessary anymore.
It is necessary for Julia.
@grosser Did you notice that my patch to the Polly build system does not call LLVMInitializeNVPTX* yet. It only ensures that the NVPTX libraries are available, which is required for any LLVMInitializeNVPTX* to not result in a link error.
This is reassuring.
Comment Actions
Sorry, I misread the last message. I understand this patch is indeed needed. I am fine for it to be committed as it is. I leave the last word to Michael, as he already started the review.