[polly] static builds

[Tobias Grosser]

On 03/13/2014 12:01 PM, Sebastian Pop wrote:
> Hi Tobi,
>
> attached is a version of the patches that link polly statically into opt. The
> patches are passing check-polly.

Very nice!

>   CMakeLists.txt                          |    2 ++
>   include/llvm/Config/config.h.cmake      |    3 +++
>   include/llvm/Config/llvm-config.h.cmake |    3 +++
>   tools/CMakeLists.txt                    |   14 ++++++++------
>   tools/bugpoint/CMakeLists.txt           |    9 +++++++++
>   tools/bugpoint/bugpoint.cpp             |   10 ++++++++++
>   tools/opt/CMakeLists.txt                |    9 +++++++++
>   tools/opt/opt.cpp                       |   10 ++++++++++
>   8 files changed, 54 insertions(+), 6 deletions(-)

This patch looks almost perfect.

> diff --git a/tools/bugpoint/bugpoint.cpp b/tools/bugpoint/bugpoint.cpp
> index b794340..5c03b41 100644
> --- a/tools/bugpoint/bugpoint.cpp
> +++ b/tools/bugpoint/bugpoint.cpp
> @@ -109,6 +109,12 @@ namespace {
>     };
>   }
>
> +#ifdef LINK_POLLY_INTO_TOOLS
> +namespace polly {
> +void initializePollyPasses(llvm::PassRegistry &Registry);
> +}
> +#endif

Is there a reason we do not include the header that defines this function?

> --- a/tools/opt/opt.cpp
> +++ b/tools/opt/opt.cpp
> @@ -304,6 +304,12 @@ static TargetMachine* GetTargetMachine(Triple TheTriple) {
>                                           GetCodeGenOptLevel());
>   }
>
> +#ifdef LINK_POLLY_INTO_TOOLS
> +namespace polly {
> +void initializePollyPasses(llvm::PassRegistry &Registry);
> +}
> +#endif

Is there a reason we do not include the header that defines this function?


> -- 1.7.6.4
>
>
> 0001-record-in-POLLY_LINK_LIBS-all-the-libs-needed-for-po.patch
>
>
>  From a44e0ff86642ccfa3b25377497f91987acb221c2 Mon Sep 17 00:00:00 2001
> From: Sebastian Pop<spop at codeaurora.org>
> Date: Thu, 13 Mar 2014 02:51:58 -0500
> Subject: [PATCH 1/2] record in POLLY_LINK_LIBS all the libs needed for polly

This one looks good. Feel free to commit it. Also, thanks for extracting 
this change into a separate commit.

> 0002-static-link-polly.patch
>
>
>  From 82e2f6c3e078fcf542b418bfbff18775db8caae8 Mon Sep 17 00:00:00 2001
> From: Sebastian Pop<spop at codeaurora.org>
> Date: Thu, 6 Mar 2014 16:32:13 -0600
> Subject: [PATCH 2/2] static link polly

I do not get this change. It seems you are restructuring a lot, but what 
are you actually doing?

>   lib/CMakeLists.txt   |   47 +++++++++++++++++++++++++++++++++--------------
>   test/lit.site.cfg.in |   15 +++++++++++++--
>   2 files changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
> index 571150e..9406816 100644
> --- a/lib/CMakeLists.txt
> +++ b/lib/CMakeLists.txt
> @@ -1,6 +1,3 @@
> -# build a monolithic libLLVMPollyLib.$shlibext
> -# and a thin module LLVMPolly.moduleext that links to that shared library
> -
>   set(LLVM_NO_RTTI 1)
>
>   if (PLUTO_FOUND)
> @@ -42,7 +39,7 @@ if (SCOPLIB_FOUND)
>         Exchange/ScopLibImporter.cpp)
>   endif (SCOPLIB_FOUND)
>
> -add_polly_library(Polly
> +set(POLLY_FILES
>     Analysis/Dependences.cpp
>     Analysis/ScopDetection.cpp
>     Analysis/ScopInfo.cpp
> @@ -74,16 +71,38 @@ add_polly_library(Polly
>     ${POLLY_PLUTO_FILES}
>     )
>
> -add_polly_loadable_module(LLVMPolly
> -  Polly.cpp
> -)
> -
> -if (TARGET intrinsics_gen)
> -  # Check if we are building as part of an LLVM build
> -  add_dependencies(LLVMPolly intrinsics_gen)
> -endif()
> -
> -target_link_libraries(LLVMPolly Polly)
> +if(LINK_POLLY_INTO_TOOLS)
> +  set(LLVM_OPTIONAL_SOURCES
> +    CodeGen/Cloog.cpp
> +    CodeGen/CodeGeneration.cpp
> +    CodeGen/PTXGenerator.cpp
> +    Exchange/OpenScopImporter.cpp
> +    Exchange/OpenScopExporter.cpp
> +    Exchange/ScopLib.cpp
> +    Exchange/ScopLibExporter.cpp
> +    Exchange/ScopLibImporter.cpp
> +    Pluto.cpp
> +    Transform/Pocc.cpp
> +    Polly.cpp)

The optional sources do not seem to be used anywhere? Are they really 
needed?

> +  add_llvm_library(LLVMPolly ${POLLY_FILES})

Is the only difference here that you call add_llvm_library instead
of add_polly_library? How are those two functions different?

The other difference is the name that changed from Polly to LLVMPolly. 
Why is this change important?

> +  if (TARGET intrinsics_gen)
> +    add_dependencies(LLVMPolly intrinsics_gen)
> +  endif()

This seems identical.

> +  if(POLLY_LINK_LIBS)
> +    foreach(lib ${POLLY_LINK_LIBS})
> +      target_link_libraries(LLVMPolly ${lib})
> +    endforeach(lib)
> +  endif(POLLY_LINK_LIBS)
> +else(LINK_POLLY_INTO_TOOLS)
> +  add_polly_library(Polly ${POLLY_FILES})
> +  if (TARGET intrinsics_gen)
> +    add_dependencies(Polly intrinsics_gen)
> +  endif()
> +  add_polly_loadable_module(LLVMPolly
> +    Polly.cpp
> +    )
> +  target_link_libraries(LLVMPolly Polly)
> +endif(LINK_POLLY_INTO_TOOLS)

The main question I ask myself, why there is any need to make this whole 
thing conditional? We already build a .a library as well as the .so 
file. Could we not just link the existing .a file into Polly?

Cheers,
Tobias