[polly] static builds

[Sebastian Pop]

Tobias Grosser wrote:
> >+#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?

I do not know how to include polly's .h files in llvm.  If you figure that out,
please let me know.  Otherwise this is the only thing we need from that file,
and we can as well add it to one of the .h files that we already include in opt.

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

These sources are optional: they are needed if you configure with CLooG, you
will need CodeGen/Cloog.cpp.  However, if you only configure with ISL, then
CMake will complain about source files present in the directory that are not
used in add_llvm_library.

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

add_polly_library is special cased in cmake/polly_macros.cmake and it never
calls add_llvm_library.  On the other hand, add_llvm_library does register the
lib as part of the LLVM libs.

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

add_llvm_library would not accept the name Polly as it has to be prefixed by LLVM.

> >+  if (TARGET intrinsics_gen)
> >+    add_dependencies(LLVMPolly intrinsics_gen)
> >+  endif()
> 
> This seems identical.

It is not identical: see below...

in the shared case, the current patch fixes it again to have the intrinsics_gen
dependence on Polly (and not on LLVMPolly): Andreas has changed it incorrectly
in this commit:

commit 7be54b8cd42d108267781c8f0ec28821caad6947
Author: Andreas Simbuerger <simbuerg at fim.uni-passau.de>
Date:   Tue Mar 11 21:26:02 2014 +0000

    (cmake) rename polly's shared lib.
    
    git-svn-id: https://llvm.org/svn/llvm-project/polly/trunk@203608 91177308-0d34-0410-b5e6-96231b3b80d8

-  add_dependencies(LLVMPollyLib intrinsics_gen)
+  add_dependencies(LLVMPolly intrinsics_gen)

This should have been:

add_dependencies(Polly intrinsics_gen)

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

...see just above.

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

Your question is then: can we switch names in the shared lib case: if we can
build a LLVMPolly.a and a Polly.so then we can factor out some of this code.

My impression is that we need an LLVMPolly.so in the case of a shared Polly, and
an LLVMPolly.a in the case we want to link polly into the tools.

Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation