[polly] r281052 - FlattenAlgo: Ensure we _really_ obtain a param space

[Tobias Grosser via llvm-commits]

[Sorry for the empty email]

On Sun, Sep 11, 2016, at 12:49 AM, Michael Kruse via llvm-commits wrote:
> The issue is that libPollyISL.a is loaded twice into the processes'
> address space.

Alright. Thanks for figuring _that_ out.
 
> Adding the code
> 
> class X {
> public:
>   X() {  printf("isl_id_none loaded at 0x%" PRIXPTR "\n", &isl_id_none);
>   }
> } x;
> 
> reveals
> 
> isl_id_none loaded at 0x7F409D769620
> isl_id_none loaded at 0x791880
> 
> (I am still wondering why they are loaded so far apart)
> Cmake runs the following commands:
> 
> [3/6] : && /usr/bin/c++ [...]  -o lib/libPolly.so [...] lib/libPollyISL.a
> [...]
> 
> [5/6] : && /usr/bin/c++ [...]  -o
> tools/polly/unittests/Flatten/FlattenTests  [...] lib/libPollyISL.a
> [...]
> 
> That is, one instance in libPolly.so, another in FlattenTests. The ISL
> commands in FlattenTest.cpp (eg. reading a set from string) will use a
> different one than the algos FlattenAlgo.cpp, because they are linked
> into different dynamic libraries.
> 
> At the moment I cannot think of an elegant solution. At very least
> cmake should not propagate linking PollyISL into everything that also
> links to Polly, ie. replacing
> 
> target_link_libraries(Polly PollyISL)
> 
> by
> 
> target_link_libraries(Polly PRIVATE PollyISL)
> 
> However, then we cannot run any ISL test anymore as eg. ISLTest.cpp is
> not allowed to call any ISL function anymore. Possible solutions I can
> currently think of:
> - Disable these unittests with BUILD_SHARED_LIBS=ON

This would make BUILD_SHARED_LIBS=ON useless for development, while this
is the only time I use this option.

> - Don't use -fvisibility=hidden

I like this one best. We introduced it once for dragonegg to work with
Polly and graphite at the same time, but this does not seem to be an
important configurations nowadways as dragonegg is clearly unmaintained.
As this feature also regularly causes trouble I feel like dropping it
for the sake of simplicity makes a lot of sense.

> - Forward the ISL calls we need for testing through libPolly:
>   isl_ctx *polly_isl_ctx_alloc() { return isl_ctx_alloc(); }

This seems to add additional complexity. I would not go for this route,
if there is not a strong need for it.

Which option would you choose?

Best,
Tobias