[polly] static builds
Tobias Grosser
tobias at grosser.es
Fri Nov 15 13:21:54 PST 2013
On 11/15/2013 09:48 PM, Sebastian Pop wrote:
> To ease review, please find the patches attached.
Hi Sebastian,
sorry for the delay.
As an overall comment, I think the intention of the patch is good and
this also seems to be in line of the feedback we got on the developer
meeting. Thanks a lot for working on this!
>>> > >diff --git a/CMakeLists.txt b/CMakeLists.txt
>>> > >index 096e3e7..895077c 100644
>>> > >--- a/CMakeLists.txt
>>> > >+++ b/CMakeLists.txt
>>> > >@@ -527,3 +527,17 @@ if(MSVC_VERSION EQUAL 1600)
>>> > > file(APPEND "${LLVM_SLN_FILENAME}" "\n# This should be regenerated!\n")
>>> > > endif()
>>> > > endif()
>>> > >+
>>> > >+option(WITH_POLLY "Build LLVM with Polly" OFF)
Why is this default to OFF? Can we make this default to on and in
addition check if polly directory exists. If people have Polly checked
out, I believe it is very likely that they would like to build it.
>>> > >+option(POLLY_BUILD_SHARED_LIB "Build Polly as a shared library (only static builds are supported under Windows)" ON)
>>> > >+if(NOT WITH_POLLY)
>>> > >+ if(LLVM_EXTERNAL_POLLY_BUILD OR NOT LLVM_EXTERNAL_POLLY_SOURCE_DIR STREQUAL "")
>>> > >+ set(WITH_POLLY ON)
>>> > >+ endif()
The two EXTERNAL* variables, are they needed in the first patch?
Otherwise, I propose to first get the basic functionality in then add
patches later if needed.
>>> > >+endif(NOT WITH_POLLY)
>>> > >+
>>> > >+if(WITH_POLLY)
>>> > >+ FIND_PACKAGE(Isl REQUIRED)
>>> > >+ FIND_PACKAGE(Gmp REQUIRED)
Is GMP really required? It seems if we build without CLooG there should
be no need for it, no? (isl may require it, but there is no direct
dependency, no?) Or is this to statically link gmp and isl?
Also, I do not see a requirement for CLooG. Does this mean Polly will be
built without CLooG if compiled into LLVM?
>>> > >+endif(WITH_POLLY)
>>> > >diff --git a/cmake/modules/FindGmp.cmake b/cmake/modules/FindGmp.cmake
>>> > >new file mode 100644
>>> > >index 0000000..3725d42
>>> > >--- /dev/null
>>> > >+++ b/cmake/modules/FindGmp.cmake
>>> > >@@ -0,0 +1,21 @@
>>> > >+FIND_PATH(GMP_INCLUDE_DIR gmp.h)
>>> > >+
>>> > >+FIND_LIBRARY(GMP_LIBRARY NAMES gmp)
>>> > >+
>>> > >+IF (POLLY_USE_GMP)
>>> > >+ IF (GMP_INCLUDE_DIR AND GMP_LIBRARY)
>>> > >+ SET(GMP_FOUND TRUE)
>>> > >+ ENDIF (GMP_INCLUDE_DIR AND GMP_LIBRARY)
>>> > >+ENDIF (POLLY_USE_GMP)
>>> > >+
>>> > >+
>>> > >+IF (GMP_FOUND)
>>> > >+ IF (NOT GMP_FIND_QUIETLY)
>>> > >+ MESSAGE(STATUS "Found GMP: ${GMP_LIBRARY}")
>>> > >+ ENDIF (NOT GMP_FIND_QUIETLY)
>>> > >+ELSE (GMP_FOUND)
>>> > >+ IF (GMP_FIND_REQUIRED)
>>> > >+ MESSAGE(FATAL_ERROR "Could not find GMP")
>>> > >+ ENDIF (GMP_FIND_REQUIRED)
>>> > >+ENDIF (GMP_FOUND)
I wonder, can we not reuse the ones already available in Polly, no?
>>> > >diff --git a/cmake/modules/FindIsl.cmake b/cmake/modules/FindIsl.cmake
>>> > >new file mode 100644
>>> > >index 0000000..bf7a0a0
>>> > >--- /dev/null
>>> > >+++ b/cmake/modules/FindIsl.cmake
>>> > >@@ -0,0 +1,27 @@
>>> > >+IF (POLLY_EXTERNAL_ISL_SOURCE_DIR)
>>> > >+ add_subdirectory(${POLLY_EXTERNAL_ISL_SOURCE_DIR} "${CMAKE_CURRENT_BINARY_DIR}/isl")
>>> > >+ IF (POLLY_BUILD_SHARED_LIB)
>>> > >+ SET(ISL_LIBRARY "${ISL_BINARY_DIR}/lib/isl.so")
>>> > >+ ELSE(POLLY_BUILD_SHARED_LIB)
>>> > >+ SET(ISL_LIBRARY "${ISL_BINARY_DIR}/lib/isl.a")
>>> > >+ ENDIF(POLLY_BUILD_SHARED_LIB)
I wonder what the best practice here is. I would assume that FindIsl
should just detect the locations of both the isl.so and the isl.a file
and at the location they are used, the correct library should then be
choosen.
>>> > >+ SET(ISL_INCLUDE_DIR "${POLLY_EXTERNAL_ISL_SOURCE_DIR}/include")
>>> > >+ INCLUDE_DIRECTORIES("${POLLY_EXTERNAL_ISL_SOURCE_DIR}/imath")
imath? Why is this needed? With the new isl interface, the math library
in isl should be completely hidden.
>>> > >+ENDIF (POLLY_EXTERNAL_ISL_SOURCE_DIR)
The whole POLLY_EXTERNAL_ISL_SOURCE_DIR seems to be an additional
feature. Maybe we can leave it out of the first code review.
>>> > >+
>>> > >+FIND_PATH(ISL_INCLUDE_DIR isl/val.h)
>>> > >+FIND_LIBRARY(ISL_LIBRARY NAMES isl)
>>> > >+
>>> > >+IF (ISL_INCLUDE_DIR AND ISL_LIBRARY)
>>> > >+ SET(ISL_FOUND TRUE)
>>> > >+ENDIF (ISL_INCLUDE_DIR AND ISL_LIBRARY)
>>> > >+
>>> > >+IF (ISL_FOUND)
>>> > >+ IF (NOT Isl_FIND_QUIETLY)
>>> > >+ MESSAGE(STATUS "Found Isl: ${ISL_LIBRARY}")
>>> > >+ ENDIF (NOT Isl_FIND_QUIETLY)
>>> > >+ELSE (ISL_FOUND)
>>> > >+ IF (Isl_FIND_REQUIRED)
>>> > >+ MESSAGE(FATAL_ERROR "Could not find Isl")
>>> > >+ ENDIF (Isl_FIND_REQUIRED)
>>> > >+ENDIF (ISL_FOUND)
The same applies here, can we use the one in Polly?
>>> > >diff --git a/lib/Transforms/IPO/PassManagerBuilder.cpp b/lib/Transforms/IPO/PassManagerBuilder.cpp
>>> > >index 0017c1b..083055f 100644
>>> > >--- a/lib/Transforms/IPO/PassManagerBuilder.cpp
>>> > >+++ b/lib/Transforms/IPO/PassManagerBuilder.cpp
>>> > >@@ -105,8 +105,18 @@ PassManagerBuilder::addInitialAliasAnalysisPasses(PassManagerBase &PM) const {
>>> > > PM.add(createBasicAliasAnalysisPass());
>>> > > }
>>> > >
>>> > >+#if defined(WITH_POLLY) && !defined(POLLY_BUILD_SHARED_LIB)
>>> > >+void registerPollyEarlyAsPossiblePasses(const llvm::PassManagerBuilder &Builder,
>>> > >+ llvm::PassManagerBase &PM);
>>> > >+void registerPollyOptLevel0Passes(const llvm::PassManagerBuilder &Builder,
>>> > >+ llvm::PassManagerBase &PM);
>>> > >+#endif // WITH_POLLY
I think including a header with those definitions would be better style.
However, my preferred solution would be to use the static initializer in
Polly.
>>> > > void PassManagerBuilder::populateFunctionPassManager(FunctionPassManager &FPM) {
>>> > > addExtensionsToPM(EP_EarlyAsPossible, FPM);
>>> > >+#if defined(WITH_POLLY) && !defined(POLLY_BUILD_SHARED_LIB)
>>> > >+ registerPollyEarlyAsPossiblePasses(*this, FPM);
>>> > >+#endif // WITH_POLLY
>>> > >
>>> > > // Add LibraryInfo if we have some.
>>> > > if (LibraryInfo) FPM.add(new TargetLibraryInfo(*LibraryInfo));
>>> > >@@ -140,6 +150,9 @@ void PassManagerBuilder::populateModulePassManager(PassManagerBase &MPM) {
>>> > > MPM.add(createBarrierNoopPass());
>>> > >
>>> > > addExtensionsToPM(EP_EnabledOnOptLevel0, MPM);
>>> > >+#if defined(WITH_POLLY) && !defined(POLLY_BUILD_SHARED_LIB)
>>> > >+ registerPollyOptLevel0Passes(*this, MPM);
>>> > >+#endif // WITH_POLLY
Is there even a need for this change? I was of the impression, that the
static initializers in Polly would register it automatically, no?
Obviously this would require PollyRegisterPasses to be linked in.
>>> > > return;
>>> > > }
>>> > >
>>> > >diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
>>> > >index 12e10fd..a3c0875 100644
>>> > >--- a/tools/CMakeLists.txt
>>> > >+++ b/tools/CMakeLists.txt
>>> > >@@ -4,6 +4,11 @@
>>> > >
>>> > > add_llvm_tool_subdirectory(llvm-config)
>>> > >
>>> > >+# Build polly before opt, as opt links against polly.
>>> > >+if(NOT POLLY_BUILD_SHARED_LIB)
>>> > >+ add_llvm_external_project(polly)
>>> > >+endif()
>>> > >+
>>> > > add_llvm_tool_subdirectory(opt)
>>> > > add_llvm_tool_subdirectory(llvm-as)
>>> > > add_llvm_tool_subdirectory(llvm-dis)
>>> > >@@ -69,7 +74,9 @@ add_llvm_external_project(clang)
>>> > > if( NOT LLVM_INCLUDE_TOOLS STREQUAL "bootstrap-only" )
>>> > > add_llvm_external_project(lld)
>>> > > add_llvm_external_project(lldb)
>>> > >- add_llvm_external_project(polly)
>>> > >+ if(POLLY_BUILD_SHARED_LIB)
>>> > >+ add_llvm_external_project(polly)
>>> > >+ endif(POLLY_BUILD_SHARED_LIB)
Can we always build Polly first? That would make the file simpler, no?
>>> > >
>>> > > # Automatically add remaining sub-directories containing a 'CMakeLists.txt'
>>> > > # file as external projects.
>>> > >--
>>> > >1.7.6.4
>>> > >
>> >
>>> > > From 7b162776f7fe52fb11a2d61ed4d1833fc1a03bcc Mon Sep 17 00:00:00 2001
>>> > >From: Sebastian Pop<spop at codeaurora.org>
>>> > >Date: Thu, 31 Oct 2013 07:17:25 -0500
>>> > >Subject: [PATCH] link polly statically
>>> > >
>>> > >---
>>> > > cmake/FindIsl.cmake | 13 ++++++-
>>> > > lib/Analysis/CMakeLists.txt | 9 ++++-
>>> > > lib/Analysis/MayAliasSet.cpp | 42 ++++++++++++++++++++++
>>> > > lib/CMakeLists.txt | 80 ++++++++++++++++++++++++++++++------------
>>> > > lib/CodeGen/CMakeLists.txt | 17 ++++++++-
>>> > > lib/Exchange/CMakeLists.txt | 19 +++++++++-
>>> > > lib/JSON/CMakeLists.txt | 7 +++-
>>> > > lib/MayAliasSet.cpp | 42 ----------------------
>>> > > lib/RegisterPasses.cpp | 32 +++++++++++-----
>>> > > lib/Support/CMakeLists.txt | 8 ++++-
>>> > > test/lit.site.cfg.in | 8 +++-
>>> > > 11 files changed, 191 insertions(+), 86 deletions(-)
>>> > > create mode 100644 lib/Analysis/MayAliasSet.cpp
>>> > > delete mode 100644 lib/MayAliasSet.cpp
>>> > >
>>> > >diff --git a/cmake/FindIsl.cmake b/cmake/FindIsl.cmake
>>> > >index 1c46e72..e3b0701 100644
>>> > >--- a/cmake/FindIsl.cmake
>>> > >+++ b/cmake/FindIsl.cmake
>>> > >@@ -1,12 +1,21 @@
>>> > >-FIND_PATH(ISL_INCLUDE_DIR isl/val.h)
>>> > >+IF (POLLY_EXTERNAL_ISL_SOURCE_DIR)
>>> > >+ add_subdirectory(${POLLY_EXTERNAL_ISL_SOURCE_DIR} "${CMAKE_CURRENT_BINARY_DIR}/isl")
>>> > >+ IF (POLLY_BUILD_SHARED_LIB)
>>> > >+ SET(ISL_LIBRARY "${ISL_BINARY_DIR}/lib/isl.so")
>>> > >+ ELSE(POLLY_BUILD_SHARED_LIB)
>>> > >+ SET(ISL_LIBRARY "${ISL_BINARY_DIR}/lib/isl.a")
>>> > >+ ENDIF(POLLY_BUILD_SHARED_LIB)
>>> > >+ SET(ISL_INCLUDE_DIR "${POLLY_EXTERNAL_ISL_SOURCE_DIR}/include")
>>> > >+ INCLUDE_DIRECTORIES("${POLLY_EXTERNAL_ISL_SOURCE_DIR}/imath")
>>> > >+ENDIF (POLLY_EXTERNAL_ISL_SOURCE_DIR)
Again, this seems to be an additional feature. I would prefer to 1st
share, this file between LLVM and Polly and 2nd, to review this in a
separate patch if possible.
>>> > >
>>> > >+FIND_PATH(ISL_INCLUDE_DIR isl/val.h)
>>> > > FIND_LIBRARY(ISL_LIBRARY NAMES isl)
>>> > >
>>> > > IF (ISL_INCLUDE_DIR AND ISL_LIBRARY)
>>> > > SET(ISL_FOUND TRUE)
>>> > > ENDIF (ISL_INCLUDE_DIR AND ISL_LIBRARY)
>>> > >
>>> > >-
>>> > > IF (ISL_FOUND)
>>> > > IF (NOT Isl_FIND_QUIETLY)
>>> > > MESSAGE(STATUS "Found Isl: ${ISL_LIBRARY}")
>>> > >diff --git a/lib/Analysis/CMakeLists.txt b/lib/Analysis/CMakeLists.txt
>>> > >index 9e46527..b40fe7b 100644
>>> > >--- a/lib/Analysis/CMakeLists.txt
>>> > >+++ b/lib/Analysis/CMakeLists.txt
>>> > >@@ -1,8 +1,15 @@
>>> > >-add_polly_library(PollyAnalysis
>>> > >+set(POLLY_ANALYSIS_SOURCES
>>> > > Dependences.cpp
>>> > >+ MayAliasSet.cpp
The move of MayAliasSet seems to be independent of this patch. The
change is trivial, but large. Please commit this already to the Polly
branch. This will make this patch set smalled.
>>> > > ScopDetection.cpp
>>> > > ScopInfo.cpp
>>> > > ScopGraphPrinter.cpp
>>> > > ScopPass.cpp
>>> > > TempScopInfo.cpp
>>> > > )
>>> > >+
>>> > >+if(POLLY_BUILD_SHARED_LIB)
>>> > >+ add_polly_library(PollyAnalysis ${POLLY_ANALYSIS_SOURCES})
>>> > >+else(POLLY_BUILD_SHARED_LIB)
>>> > >+ add_llvm_library(LLVMPollyAnalysis ${POLLY_ANALYSIS_SOURCES})
Can you help me what the difference here is again? Maybe add a comment
to explain.
Would it make sense to always use LLVMPollyAnalysis?
79c5ab 100644
>>> > >--- a/lib/CMakeLists.txt
>>> > >+++ b/lib/CMakeLists.txt
>>> > >@@ -4,31 +4,30 @@ add_subdirectory(Exchange)
>>> > > add_subdirectory(Support)
>>> > > add_subdirectory(JSON)
>>> > >
>>> > >-set(MODULE TRUE)
>>> > >+if(POLLY_BUILD_SHARED_LIB)
>>> > >+ set(MODULE TRUE)
>>> > >+endif(POLLY_BUILD_SHARED_LIB)
>>> > > set(LLVM_NO_RTTI 1)
>>> > >
>>> > > if (SCOPLIB_FOUND)
>>> > > set(POLLY_SCOPLIB_FILES Pocc.cpp)
>>> > >+elseif(NOT POLLY_BUILD_SHARED_LIB)
>>> > >+ set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES}
>>> > >+ Pocc.cpp)
>>> > > endif (SCOPLIB_FOUND)
>>> > >
>>> > > if (PLUTO_FOUND)
>>> > > set(POLLY_PLUTO_FILES Pluto.cpp)
>>> > >+elseif(NOT POLLY_BUILD_SHARED_LIB)
>>> > >+ set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES}
>>> > >+ Pluto.cpp)
>>> > > endif (PLUTO_FOUND)
This change looks also fishy. Can we avoid these conditions.
>>> > >-if (TARGET intrinsics_gen)
>>> > >- # Check if we are building as part of an LLVM build
>>> > >- add_dependencies(LLVMPolly intrinsics_gen)
>>> > >-endif()
>>> > >-
>>> > >-add_dependencies(LLVMPolly
>>> > >- PollyAnalysis
>>> > >- PollyCodeGen
>>> > >- PollyExchange
>>> > >- PollySupport
>>> > >- PollyJSON
>>> > >- )
>>> > >+if(POLLY_BUILD_SHARED_LIB)
>>> > >+ set(LLVM_USED_LIBS
>>> > >+ PollyAnalysis
>>> > >+ PollyCodeGen
>>> > >+ PollyExchange
>>> > >+ PollySupport
>>> > >+ PollyJSON
>>> > >+ )
>>> > >+ add_polly_loadable_module(LLVMPolly ${POLLY_SRC})
>>> > >+
>>> > >+ if (TARGET intrinsics_gen)
>>> > >+ # Check if we are building as part of an LLVM build
>>> > >+ add_dependencies(LLVMPolly intrinsics_gen)
>>> > >+ endif()
>>> > >+
>>> > >+ add_dependencies(LLVMPolly
>>> > >+ PollyAnalysis
>>> > >+ PollyCodeGen
>>> > >+ PollyExchange
>>> > >+ PollySupport
>>> > >+ PollyJSON
>>> > >+ )
>>> > >+else(POLLY_BUILD_SHARED_LIB)
>>> > >+ set(LLVM_USED_LIBS
>>> > >+ LLVMPollyAnalysis
>>> > >+ LLVMPollyCodeGen
>>> > >+ LLVMPollyExchange
>>> > >+ LLVMPollySupport
>>> > >+ LLVMPollyJSON
>>> > >+ )
Before we add complexity to the makefiles, I would rather prefer to
rename everything to LLVMPolly*. Such a commit (if done for make system
and cmake) is preapproved for Polly.
>>> > >+ add_llvm_library(LLVMPolly ${POLLY_SRC})
>>> > >+
>>> > >+ if (TARGET intrinsics_gen)
>>> > >+ # Check if we are building as part of an LLVM build
>>> > >+ add_dependencies(LLVMPolly intrinsics_gen)
>>> > >+ endif()
>>> > >+
>>> > >+ add_dependencies(LLVMPolly
>>> > >+ LLVMPollyAnalysis
>>> > >+ LLVMPollyCodeGen
>>> > >+ LLVMPollyExchange
>>> > >+ LLVMPollySupport
>>> > >+ LLVMPollyJSON
>>> > >+ )
>>> > >+
>>> > >+ target_link_libraries(LLVMPolly LLVMTransformUtils LLVMAnalysis)
>>> > >+endif(POLLY_BUILD_SHARED_LIB)
>>> > > set_target_properties(LLVMPolly
>>> > > PROPERTIES
>>> > >diff --git a/lib/CodeGen/CMakeLists.txt b/lib/CodeGen/CMakeLists.txt
>>> > >index b1c29c5..39c72b0 100644
>>> > >--- a/lib/CodeGen/CMakeLists.txt
>>> > >+++ b/lib/CodeGen/CMakeLists.txt
>>> > >@@ -2,6 +2,10 @@ if (CLOOG_FOUND)
>>> > > set(CLOOG_FILES
>>> > > Cloog.cpp
>>> > > CodeGeneration.cpp)
>>> > >+elseif(NOT POLLY_BUILD_SHARED_LIB)
>>> > >+ set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES}
>>> > >+ Cloog.cpp
>>> > >+ CodeGeneration.cpp)
>>> > > endif (CLOOG_FOUND)
>>> > >
>>> > > set(ISL_CODEGEN_FILES
>>> > >@@ -11,13 +15,22 @@ set(ISL_CODEGEN_FILES
>>> > > if (GPU_CODEGEN)
>>> > > set (GPGPU_CODEGEN_FILES
>>> > > PTXGenerator.cpp)
>>> > >+elseif(NOT POLLY_BUILD_SHARED_LIB)
>>> > >+ set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES}
>>> > >+ PTXGenerator.cpp)
>>> > > endif (GPU_CODEGEN)
Sorry, I am a little bit in a rush, but these conditions all over the
place look surprising. I need to look again into why they are needed.
>>> > >-add_polly_library(PollyCodeGen
>>> > >+set(POLLY_CODEGEN_SOURCES
>>> > > BlockGenerators.cpp
>>> > > ${CLOOG_FILES}
>>> > > ${ISL_CODEGEN_FILES}
>>> > > LoopGenerators.cpp
>>> > > Utils.cpp
>>> > > ${GPGPU_CODEGEN_FILES}
>>> > >-)
>>> > >+ )
>>> > >+
>>> > >+if(POLLY_BUILD_SHARED_LIB)
>>> > >+ add_polly_library(PollyCodeGen ${POLLY_CODEGEN_SOURCES})
>>> > >+else(POLLY_BUILD_SHARED_LIB)
>>> > >+ add_llvm_library(LLVMPollyCodeGen ${POLLY_CODEGEN_SOURCES})
>>> > >+endif(POLLY_BUILD_SHARED_LIB)
And again, what is the difference here. If we really need to switch
behavior, we should probably adjust the behavior of add_polly_library
based on POLLY_BUILD_SHARED_LIB?
>>> > >diff --git a/lib/RegisterPasses.cpp b/lib/RegisterPasses.cpp
>>> > >index 26a138e..bca1cf2 100644
>>> > >--- a/lib/RegisterPasses.cpp
>>> > >+++ b/lib/RegisterPasses.cpp
>>> > >@@ -310,12 +310,9 @@ static void registerPollyPasses(llvm::PassManagerBase &PM) {
>>> > > PM.add(llvm::createCFGPrinterPass());
>>> > > }
>>> > >
>>> > >-static void
>>> > >-registerPollyEarlyAsPossiblePasses(const llvm::PassManagerBuilder &Builder,
>>> > >- llvm::PassManagerBase &PM) {
>>> > >-
>>> > >- if (Builder.OptLevel == 0)
>>> > >- return;
>>> > >+static bool shouldEnablePolly(unsigned OptLevel) {
>>> > >+ if (OptLevel == 0)
>>> > >+ return false;
>>> > >
>>> > > if (PollyOnlyPrinter || PollyPrinter || PollyOnlyViewer || PollyViewer)
>>> > > PollyTrackFailures = true;
>>> > >@@ -324,7 +321,16 @@ registerPollyEarlyAsPossiblePasses(const llvm::PassManagerBuilder &Builder,
>>> > > ExportJScop || ImportJScop)
>>> > > PollyEnabled = true;
>>> > >
>>> > >- if (!PollyEnabled)
>>> > >+ return PollyEnabled;
>>> > >+}
>>> > >+
>>> > >+} // end of anonymous namespace.
Please commit shouldEnablePolly separately. This is a trivial, makes the
review easier and is preapproved.
>>> > >+
>>> > >+void
>>> > >+registerPollyEarlyAsPossiblePasses(const llvm::PassManagerBuilder &Builder,
>>> > >+ llvm::PassManagerBase &PM) {
>>> > >+
>>> > >+ if (!shouldEnablePolly(Builder.OptLevel))
>>> > > return;
>>> > >
>>> > > // We only run Polly at optimization level '-O3'.
>>> > >@@ -340,11 +346,15 @@ registerPollyEarlyAsPossiblePasses(const llvm::PassManagerBuilder &Builder,
>>> > > registerPollyPasses(PM);
>>> > > }
>>> > >
>>> > >-static void registerPollyOptLevel0Passes(const llvm::PassManagerBuilder &,
>>> > >- llvm::PassManagerBase &PM) {
>>> > >- registerCanonicalicationPasses(PM);
>>> > >+void registerPollyOptLevel0Passes(const llvm::PassManagerBuilder &Builder,
>>> > >+ llvm::PassManagerBase &PM) {
>>> > >+ if (shouldEnablePolly(Builder.OptLevel))
>>> > >+ registerCanonicalicationPasses(PM);
>>> > > }
Making those functions non-static without exposing them in a header
looks like a hack ...
>>> > >+#ifdef POLLY_BUILD_SHARED_LIB
>>> > >+namespace {
>>> > >+
>>> > > /// @brief Register Polly canonicalization passes at opt level '0'
>>> > > ///
>>> > > /// At '-O0' we schedule the Polly canonicalization passes. This allows us
>>> > >@@ -385,3 +395,5 @@ static llvm::RegisterStandardPasses
>>> > > RegisterPollyOptimizer(llvm::PassManagerBuilder::EP_EarlyAsPossible,
>>> > > registerPollyEarlyAsPossiblePasses);
>>> > > } // end of anonymous namespace.
>>> > >+
>>> > >+#endif // POLLY_BUILD_SHARED_LIB
This static initializer can not be used to register the Polly passes
even in a statically linked Polly?
>>> > >diff --git a/test/lit.site.cfg.in b/test/lit.site.cfg.in
>>> > >index d87f2fe..7abed4a 100644
>>> > >--- a/test/lit.site.cfg.in
>>> > >+++ b/test/lit.site.cfg.in
>>> > >@@ -25,8 +25,12 @@ except KeyError,e:
>>> > > key, = e.args
>>> > > lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
>>> > >
>>> > >-config.substitutions.append(('%loadPolly', '-load '
>>> > >- + config.polly_lib_dir + '/LLVMPolly at LLVM_SHLIBEXT@'))
>>> > >+if '@POLLY_BUILD_SHARED_LIB@' == 'true':
>>> > >+ config.substitutions.append(('%loadPolly', '-load '
>>> > >+ + config.polly_lib_dir + '/LLVMPolly at LLVM_SHLIBEXT@'))
>>> > >+else:
>>> > >+ config.substitutions.append(('%loadPolly', ''))
>>> > >+
>>> > > config.substitutions.append(('%defaultOpts', ' -basicaa -polly-prepare'))
>>> > > config.substitutions.append(('%polybenchOpts', ' -O3 -loop-simplify -indvars '))
>>> > > config.substitutions.append(('%vector-opt', '-polly-vectorizer=polly'))
Thanks a lot for your work.
I think getting this in is very valuable. I will later tonight or
tomorrow have another look, but wanted to give some feedback quickly.
Cheers,
Tobias
More information about the llvm-commits
mailing list