USE_POLLY_ACC : Remove dependency on CUDA headerfiles and link libjulia to libGPURuntime #22036

singam-sanjay · 2017-05-23T11:13:34Z

Remove the dependency on CUDA header files and link libGPURuntime to libjulia for USE_POLLY_ACC = 1.
Also move libGPURuntime.so to usr/lib instead of adding rpath to its original location.

…ia to libGPURuntime Move libGPURuntime.so to usr/lib instead of adding rpath to its original location.

ararslan · 2017-05-23T20:43:52Z

src/Makefile

+ifeq ($(OS), WINNT)
+ @cp $(shell $(LLVM_CONFIG_HOST) --libdir)/libGPURuntime.dll $(build_shlibdir)
+else
+ @cp $(shell $(LLVM_CONFIG_HOST) --libdir)/libGPURuntime.$(SHLIB_EXT) $(build_shlibdir)


You shouldn't need the conditional here, since $(SHLIB_EXT) will be dll on Windows

grep SHLIB_EXT= -Rsn gave me this,

contrib/fixup-libgfortran.sh:21: SHLIB_EXT="so" contrib/fixup-libgfortran.sh:23: SHLIB_EXT="dylib" contrib/fixup-libgfortran.sh:27: SHLIB_EXT="so"

So, I thought I had to handle the case for Windows. Anyways, I doubt that fixup-libgfortran.sh is setting the SHLIB_EXT used in src/Makefile. How is it getting set then ?

SHLIB_EXT is set somewhere around line 500 of Make.inc. Make.inc is the file that defines the standard stuff that's used in all of the various makefiles, SHLIB_EXT being one example.

In this case, the variable is set using the eager resolution assignment operator, :=. Regular = does lazy resolution in Make. It also has a space before the assignment. So grepping with a trailing = won't find it. Just now I found it with sift -e '\bSHLIB_EXT\s*[:?]?=' -n Make.inc.

tkelman · 2017-05-24T05:59:10Z

src/Makefile

@@ -278,6 +274,10 @@ ifneq ($(OS), WINNT)
 @ln -sf libjulia-debug.$(JL_MAJOR_MINOR_SHLIB_EXT) $(build_shlibdir)/libjulia-debug.$(JL_MAJOR_SHLIB_EXT)
 @ln -sf libjulia-debug.$(JL_MAJOR_MINOR_SHLIB_EXT) $(build_shlibdir)/libjulia-debug.$(SHLIB_EXT)
 endif
+ifeq ($(USE_POLLY_ACC), 1)
+# Place libGPURuntime such that it's accessible under libjulia's RPATH
+ @cp $(shell $(LLVM_CONFIG_HOST) --libdir)/libGPURuntime.$(SHLIB_EXT) $(build_shlibdir)


this isn't the right target for this - LLVM should be responsible for installing this library if it's part of the public interface

libGPURuntime is a part of Polly and not LLVM, therefore LLVM will not handling it.

if it's part of the public interface

what is a "public interface" ?

I mean if this library is meant to be linked to externally, and isn't just a temporary byproduct of the polly build process, then polly's build system should be installing it when you do make install on polly.

libGPURuntime is installed into usr/lib when Julia's using an LLVM build in deps/scratch, but needs to be copied when an external LLVM build is being used.

where would it get installed for an external LLVM build? wouldn't it be in the same place as the rest of the llvm-config -L linking flags already cover?

lt would be available in llvm-config --libdir.

I'm copying it from --libdir. libGPURuntime.so would be first written to usr/lib by LLVM_INSTALL and be overwritten with the same content by this copy command.

@cp $(shell $(LLVM_CONFIG_HOST) --libdir)/libGPURuntime.$(SHLIB_EXT) $(build_shlibdir)

this isn't the target for libGPURuntime, this is the target for libjulia-debug, so why is it being copied in this step? and for external-llvm builds, why does it need to be copied at all?

libGPURuntime.so is required to run the PTX code generated by Polly-ACC, therefore has to be accessible to libjulia.so or libjulia-debug.so's RPATH. Previously, I managed this by hard-coding the location of the library through the -rpath, which was discouraged by @vtjnash in this comment.

This meant that I had to place libGPURuntime.so into a directory covered by the default RPATH, $ORIGIN:$ORIGIN/julia. So, I'm copying libGPURuntime to usr/lib ($ORIGIN w.r.t. libjulia*.so).

and for external-llvm builds, why does it need to be copied at all?

All libraries are installed in usr/lib only when using an internal-llvm build. So it has to be copied in case of external-llvm builds.

this isn't the target for libGPURuntime, this is the target for libjulia-debug, so why is it being copied in this step?

Actually, it has to be copied into usr/lib for both libjulia and libjulia-debug. I made a mistake by not adding the copy command in the case of libjulia also. I'll add a libGPURuntime target and make both libjulia and libjulia-target depend on it.

tkelman · 2017-05-24T12:12:42Z

src/Makefile

 endif
 endif
 else
 SRCS += anticodegen
 LLVM_LIBS := support
 endif

+$(build_shlibdir)/libGPURuntime.so:


.$(SHLIB_EXT), not .so

Thanks for pointing it out ! Corrected by the next commit.

tkelman · 2017-06-03T21:20:06Z

why does this library need to be copied when using system llvm? it's a bit strange to piecemeal move around only some so files after they've been compiled, without doing any post move fixups (with patchelf or similar)

staticfloat · 2017-06-03T23:29:23Z

src/Makefile

+ifeq ($(USE_POLLY_ACC), 1)
+# Place libGPURuntime such that it's accessible under libjulia's RPATH
+ @cp $(shell $(LLVM_CONFIG_HOST) --libdir)/libGPURuntime.$(SHLIB_EXT) $(build_shlibdir)
+endif


If I'm understanding the purpose of this line properly, I think this should be invoked via the install-llvm target within deps/llvm.mk, rather than as a standalone target within src/Makefile. We should treat this library the same as we treat the rest of the LLVM libraries, so that it gets installed at the same time, patched via patchelf at the same time, etc...

see some collapsed discussion above, I think the reason it wasn't done that way is to account for system llvm

So then what do we do for libLLVM.so when using system LLVM?

@singam-sanjay Why do we need to copy this library in when libLLVM.so itself doesn't get copied in?

@staticfloat When building Julia with an external/system LLVM-Polly build, USE_LLVM_SHLIB:=0 is a default setting.

So then what do we do for libLLVM.so when using system LLVM?

libjulia is instead linked to the static libraries.

@singam-sanjay Why do we need to copy this library in when libLLVM.so itself doesn't get copied in?

Even when not linking with libLLVM.so, GPURuntime has to be made available whenever USE_POLLY_ACC:=1.

Please note that julia needs to be built with llvm-svn whenever USE_POLLY:=1, so the above arguments may not hold for system LLVM made available through a package manager.

Personally, I'm not sure we include support for USE_SYSTEM_* configurations that aren't from distributions. This seems like a custom setup that another user may not necessarily replicate properly. I'm more inclined to support either a build from within Julia's source tree, or only binaries sourced from package managers.

@singam-sanjay Am I correct in assuming that your usecase is that you have an external LLVM already built to specifications, and the reason you want this functionality is because you want to avoid rebuilding LLVM within Julia's source tree?

Not just that, It would be beneficial while developing Julia and LLVM (and Polly ;-)). This is what I have,

$ ls julia_src llvm_build llvm_src polly_src

Some of the benefits I've felt,

Helps me to be mindful of each step of the build. e.g I know that issuing a make in julia_src/ will not build llvm.

If the LLVM and Polly repos are going to used to submit patches, having them outside the julia source-tree would be better.

@staticfloat ping.

The spirit of USE_SYSTEM_XYZ flags is to allow for users to use prebuilt binaries of external libraries. E.g. to be able to run USE_SYSTEM_LLVM=1 in order to use the LLVM libraries that you get when you apt install llvm-dev, or similar. We don't really intend for it to be used with from-source builds that users create outside of the source tree. This is because the number of possible states that users can get these out-of-source builds into is very large, and dealing with them all intelligently from within the Julia build system is nigh-impossible. We'd much rather restrict the build system to handle only two cases; builds from within the Julia source tree, and re-using prebuilt binaries from system package managers. As there are no system package managers that ship with the features that you're interested in at the moment, I would encourage you to do your development within the LLVM that is managed within the Julia build tree. Any patches you are interested in adding to Julia to spice up the LLVM build recipes to get it built the way you want, I will be happy to review and merge, as this will make it a lot easier for others to replicate your work and build Julia with those features enabled.

So at this point, I'm going to have to say that I don't think I'm willing to merge these changes, I'd like to push this effort toward getting the build incantations for the Julia-owned LLVM to the point where you are happy with the result and can build the LLVM you want Julia to use there, and then we can work on making that portable enough that anyone can build that spiced-up version of LLVM.

staticfloat · 2017-06-10T22:40:18Z

@singam-sanjay is this cut-down patch enough for you to compile Polly as you need it on your system?

singam-sanjay · 2017-06-11T18:13:11Z

@singam-sanjay is this cut-down patch enough for you to compile Polly as you need it on your system?

Yes, it's enough.

tkelman · 2017-06-11T19:14:33Z

This simpler version as of c859a37 looks okay to me, though it should be squashed

staticfloat · 2017-06-11T19:34:50Z

If you do make install do all the necessary libraries get copied into the installation prefix so that you can copy that tarball around?

singam-sanjay · 2017-06-12T07:38:15Z

If you do make install do all the necessary libraries get copied into the installation prefix

Yes, the required libraries are copied into usr/lib for both make and make install. What I also found was that those of the libraries which were .so files were marked with permissions 775 in deps/scratch/llvm-svn and marked 644 in usr/lib.

➜  lib git:(rm_CUDALIB_dependency-rebased) ll libLLVM-5.0svn.so libGPURuntime.so 
-rw-r--r-- 1 sanjay sanjay 24K Jun 11 23:26 libGPURuntime.so
-rw-r--r-- 1 sanjay sanjay 37M Jun 11 23:32 libLLVM-5.0svn.so
➜  lib git:(rm_CUDALIB_dependency-rebased) cd ../../deps/scratch/llvm-svn/build_Release/lib 
➜  lib git:(rm_CUDALIB_dependency-rebased) ll libLLVM-5.0svn.so libGPURuntime.so
-rwxrwxr-x 1 sanjay sanjay 24K Jun 11 23:26 libGPURuntime.so
-rwxrwxr-x 1 sanjay sanjay 37M Jun 11 23:32 libLLVM-5.0svn.so

I don't know how simply copying can effect such a change. Would that cause a problem later on ?

The variables that informed CMake of the install directory were actually not expected by it,

[CMakeCache.txt]
...
//No help, variable specified on the command line.
LIB_INSTALL_DIR:UNINITIALIZED=/home/sanjay/Software/julia/julia_src/usr/lib
...
//No help, variable specified on the command line.
CMAKE_INSTALL_LIBDIR:UNINITIALIZED=/home/sanjay/Software/julia/julia_src/usr/lib

How did the libraries end up in usr/lib if CMake wasn't going to use these variables ?

so that you can copy that tarball around?

As in, a tarball enclosing everything in <julia_src>/usr/ ?

staticfloat · 2017-06-13T04:46:25Z

If you run make binary-dist, it makes a tarball out of the results of make install. I just want to see if this distribution of Julia is portable; to ensure that all the relevant libraries and such are installed properly.

singam-sanjay · 2017-06-13T08:50:38Z

Yes it does. I've got <julia_src>/julia-de6b8a2019-Linux-x86_64.tar.gz after running make binary-dist.

But, I couldn't find libGPURuntime.so under <tar>/lib or <tar>/lib/julia.

singam-sanjay · 2017-06-13T14:17:16Z

@staticfloat Thanks for the merge !

I couldn't find libGPURuntime.so under <tarball>/lib or <tarball>/lib/julia. Is that fine ? Does it need to be handled ?

tkelman · 2017-06-13T19:28:26Z

is it under usr/lib in the source build tree?

singam-sanjay · 2017-06-13T19:40:25Z

is it under usr/lib in the source build tree?

Yes, libGPURuntime.so ( and other Polly static and dynamic libraries ) was copied to <julia_src>/usr/lib.

tkelman · 2017-06-13T19:46:58Z

then probably it needs to be added to a list in our make install target

singam-sanjay · 2017-06-14T08:43:29Z

I'm assuming that libGPURuntime.so should be added to install's list and not binary-dist's list because install is invoked in binary-dist at,
443: @$(MAKE) -C $(BUILDROOT) -f $(JULIAHOME)/Makefile install

But make install already copies libGPURuntime.so into usr/lib. How does this behaviour change when it's executed inside $(BUILDROOT) ?

singam-sanjay · 2017-06-14T13:50:19Z

then probably it needs to be added to a list in our make install target

@tkelman @staticfloat I've updated the branch to include GPURuntime in JL_PRIVATE_LIBS. I've opened a new PR #22365 to address this.

…ia to libGPURuntime (#22036) * USE_POLLY_ACC : Remove dependency on CUDA headerfiles and link libjulia to libGPURuntime * Removed comments and CUDALIB_INCLUDE_DIR variable.

Summary: Polly can now offload Julia to GPUs. This was enabled by the pull requests JuliaLang/julia#21736 and JuliaLang/julia#22036. Reviewers: grosser, bollu Subscribers: pollydev Tags: #polly Differential Revision: https://reviews.llvm.org/D36050 git-svn-id: https://llvm.org/svn/llvm-project/polly/branches/release_50@309656 91177308-0d34-0410-b5e6-96231b3b80d8

Summary: Polly can now offload Julia to GPUs. This was enabled by the pull requests JuliaLang/julia#21736 and JuliaLang/julia#22036. Reviewers: grosser, bollu Subscribers: pollydev Tags: #polly Differential Revision: https://reviews.llvm.org/D36050 llvm-svn=309656

Summary: Polly can now offload Julia to GPUs. This was enabled by the pull requests JuliaLang/julia#21736 and JuliaLang/julia#22036. Reviewers: grosser, bollu Subscribers: pollydev Tags: #polly Differential Revision: https://reviews.llvm.org/D36050 llvm-svn: 309656

singam-sanjay added 3 commits May 23, 2017 15:59

USE_POLLY_ACC : Remove dependency on CUDA headerfiles and link libjul…

8282740

…ia to libGPURuntime Move libGPURuntime.so to usr/lib instead of adding rpath to its original location.

Removed comments and CUDALIB_INCLUDE_DIR variable.

b357f35

Comments on libGPURuntime copy

6f79b1a

ararslan reviewed May 23, 2017

View reviewed changes

Removed handling SHLIB_EXT for Windows and updated comment

727e37f

tkelman reviewed May 24, 2017

View reviewed changes

singam-sanjay added 2 commits May 24, 2017 16:46

libGPURuntime is now a dependency of libjuilia and libjulia-debug.

c52c7e4

Remove trailing whitespace

e85ac53

tkelman reviewed May 24, 2017

View reviewed changes

Corrected libGPURuntime.so to libGPURuntime.$(SHLIB_EXT)

95b0719

ararslan requested a review from staticfloat June 3, 2017 19:08

staticfloat reviewed Jun 3, 2017

View reviewed changes

Remove explicitly copying libGPURuntime.so

c859a37

ViralBShah added the domain:building Build system, or building Julia or its dependencies label Jun 11, 2017

staticfloat merged commit 70be8ab into JuliaLang:master Jun 13, 2017

singam-sanjay mentioned this pull request Jun 14, 2017

Ensure libGPURuntime.so is a part of the portable tarball for USE_POLLY_ACC #22365

Closed

USE_POLLY_ACC : Remove dependency on CUDA headerfiles and link libjulia to libGPURuntime #22036

USE_POLLY_ACC : Remove dependency on CUDA headerfiles and link libjulia to libGPURuntime #22036

Conversation

singam-sanjay commented May 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman commented Jun 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticfloat commented Jun 10, 2017

singam-sanjay commented Jun 11, 2017

tkelman commented Jun 11, 2017

staticfloat commented Jun 11, 2017

singam-sanjay commented Jun 12, 2017

staticfloat commented Jun 13, 2017

singam-sanjay commented Jun 13, 2017 • edited Loading

singam-sanjay commented Jun 13, 2017 • edited Loading

tkelman commented Jun 13, 2017

singam-sanjay commented Jun 13, 2017

tkelman commented Jun 13, 2017

singam-sanjay commented Jun 14, 2017

singam-sanjay commented Jun 14, 2017 • edited Loading

singam-sanjay commented Jun 13, 2017 •

edited

Loading

singam-sanjay commented Jun 13, 2017 •

edited

Loading

singam-sanjay commented Jun 14, 2017 •

edited

Loading