Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 8 commits into from
Jun 13, 2017
Merged

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8282740
USE_POLLY_ACC : Remove dependency on CUDA headerfiles and link libjul…
singam-sanjay May 23, 2017
b357f35
Removed comments and CUDALIB_INCLUDE_DIR variable.
singam-sanjay May 23, 2017
6f79b1a
Comments on libGPURuntime copy
singam-sanjay May 23, 2017
727e37f
Removed handling SHLIB_EXT for Windows and updated comment
singam-sanjay May 24, 2017
c52c7e4
libGPURuntime is now a dependency of libjuilia and libjulia-debug.
singam-sanjay May 24, 2017
e85ac53
Remove trailing whitespace
singam-sanjay May 24, 2017
95b0719
Corrected libGPURuntime.so to libGPURuntime.$(SHLIB_EXT)
singam-sanjay May 24, 2017
c859a37
Remove explicitly copying libGPURuntime.so
singam-sanjay Jun 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Make.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ BUILD_LLDB := 0
USE_POLLY := 0
USE_POLLY_OPENMP := 0 # Enable OpenMP code-generation
USE_POLLY_ACC := 0 # Enable GPU code-generation
CUDALIB_INCLUDE_DIR := # Path to CUDA header files

# Cross-compile
#XC_HOST := i686-w64-mingw32
Expand Down
18 changes: 10 additions & 8 deletions src/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,22 @@ ifeq ($(USE_POLLY_OPENMP),1)
FLAGS += -fopenmp
endif
ifeq ($(USE_POLLY_ACC),1)
LLVMLINK += -lPollyPPCG
LLVMLINK += -lPollyPPCG -lGPURuntime
FLAGS += -DUSE_POLLY_ACC
ifeq ($(CUDALIB_INCLUDE_DIR),)
$(error CUDALIB_INCLUDE_DIR not set: If compiling with USE_POLLY_ACC, user must provide the path to CUDA header files)
endif
FLAGS += -I$(CUDALIB_INCLUDE_DIR)
FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --src-root)/tools/polly/tools
FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --src-root)/tools/polly/tools # Required to find GPURuntime/GPUJIT.h
endif
endif
else
SRCS += anticodegen
LLVM_LIBS := support
endif

$(build_shlibdir)/libGPURuntime.$(SHLIB_EXT):
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

  1. 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.
  2. If the LLVM and Polly repos are going to used to submit patches, having them outside the julia source-tree would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staticfloat ping.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# headers are used for dependency tracking, while public headers will be part of the dist
HEADERS := $(BUILDDIR)/julia_version.h $(wildcard $(SRCDIR)/support/*.h) $(addprefix $(SRCDIR)/,julia.h julia_threads.h julia_internal.h options.h timing.h)
PUBLIC_HEADERS := $(BUILDDIR)/julia_version.h $(wildcard $(SRCDIR)/support/*.h) $(addprefix $(SRCDIR)/,julia.h julia_threads.h)
Expand Down Expand Up @@ -271,7 +273,7 @@ else
SONAME_DEBUG :=
endif

$(build_shlibdir)/libjulia-debug.$(JL_MAJOR_MINOR_SHLIB_EXT): $(SRCDIR)/julia.expmap $(DOBJS) $(BUILDDIR)/flisp/libflisp-debug.a $(BUILDDIR)/support/libsupport-debug.a $(LIBUV)
$(build_shlibdir)/libjulia-debug.$(JL_MAJOR_MINOR_SHLIB_EXT): $(SRCDIR)/julia.expmap $(DOBJS) $(BUILDDIR)/flisp/libflisp-debug.a $(BUILDDIR)/support/libsupport-debug.a $(LIBUV) $(build_shlibdir)/libGPURuntime.$(SHLIB_EXT)
@$(call PRINT_LINK, $(CXXLD) $(CXXFLAGS) $(CXXLDFLAGS) $(DEBUGFLAGS) $(DOBJS) $(RPATH_LIB) -o $@ $(LDFLAGS) $(JLIBLDFLAGS) $(DEBUG_LIBS) $(SONAME_DEBUG))
$(INSTALL_NAME_CMD)libjulia-debug.$(SHLIB_EXT) $@
ifneq ($(OS), WINNT)
Expand All @@ -286,7 +288,7 @@ $(BUILDDIR)/libjulia-debug.a: $(SRCDIR)/julia.expmap $(DOBJS) $(BUILDDIR)/flisp/

libjulia-debug: $(build_shlibdir)/libjulia-debug.$(JL_MAJOR_MINOR_SHLIB_EXT) $(PUBLIC_HEADER_TARGETS)

$(build_shlibdir)/libjulia.$(JL_MAJOR_MINOR_SHLIB_EXT): $(SRCDIR)/julia.expmap $(OBJS) $(BUILDDIR)/flisp/libflisp.a $(BUILDDIR)/support/libsupport.a $(LIBUV)
$(build_shlibdir)/libjulia.$(JL_MAJOR_MINOR_SHLIB_EXT): $(SRCDIR)/julia.expmap $(OBJS) $(BUILDDIR)/flisp/libflisp.a $(BUILDDIR)/support/libsupport.a $(LIBUV) $(build_shlibdir)/libGPURuntime.$(SHLIB_EXT)
@$(call PRINT_LINK, $(CXXLD) $(CXXFLAGS) $(CXXLDFLAGS) $(SHIPFLAGS) $(OBJS) $(RPATH_LIB) -o $@ $(LDFLAGS) $(JLIBLDFLAGS) $(RELEASE_LIBS) $(SONAME))
$(INSTALL_NAME_CMD)libjulia.$(SHLIB_EXT) $@
ifneq ($(OS), WINNT)
Expand Down