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.
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
WIP: Integrate Polly and make it available via @polly
macro
#16531
Conversation
Hi Matthias, thanks for pushing this ahead. From the Polly side, the architecture of this patch looks right. It connects all of Polly to Julia, while being minimal-intrusive. With this, we can start to increase the coverage and work on first optimizations. I am interested to hear what the Julia side thinks of this patch. |
Surprisingly non-disruptive, a nice start. Will |
As far as testing goes, perhaps something like function looporder1(A::Matrix)
s = zero(eltype(A))
@inbounds for i = 1:size(A,1), j = 1:size(A,2) # deliberately in cache-inefficient order
s += A[i,j]
end
end
@polly function looporder2(A::Matrix)
... # same function body as above
end and then test that |
e4402ef
to
fd5554e
Compare
Thank you for the feedback. For now, I don't expect any additions to |
I now extended this to enable the build to automatically retrieve Polly, like mentioned above. So, if one is interested in trying these features it's sufficient to have the following in
|
BUILD_LLVM_CLANG := 1 # because it's a build requirement | ||
ifeq ($(USE_SYSTEM_LLVM),0) | ||
ifneq ($(LLVM_VER),svn) | ||
$(error USE_POLLY=1 requires LLVM_VER=svn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only because you haven't hooked up the tarball yet, right? I would hold off on that and make it a separate PR.
pulling from polly should probably be added to the update-llvm rule at the very end of this file though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this will probably be better off on a separate pull request. Yes, since my changes on the Polly side will only be available in the new version (therefore not yet available as tarball), I don't know if it's meaningful to download older versions here. (and thank you for the hint with the update-llvm rule).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tkelman Just to make sure that I didn't get you wrong: do you think I should just wait with this functionality until the tarball is available, or is it okay if I already open a new PR for this functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go ahead I'd say. It may be some time before Julia uses LLVM 3.9 by default. As long as your options are off by default initially they can be a bit unreliable or temporary to start with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you for the clarification.
9ca3267
to
38152bb
Compare
@@ -105,6 +105,10 @@ | |||
#if defined(_CPU_ARM_) || defined(_CPU_AARCH64_) | |||
# include <llvm/IR/InlineAsm.h> | |||
#endif | |||
#if defined(USE_POLLY) | |||
#include <polly/RegisterPasses.h> | |||
#include <polly/ScopDetection.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now going to think "Scop" is a typo every time I see it. "Static control part," apparently? https://github.com/llvm-mirror/polly/blob/master/include/polly/ScopDetection.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct
This is a first attempt to integrate the polyhedral optimizer Polly into Julia in the context of my GSoC project. Since (for now) it is not yet clear whether it's beneficial to turn on Polly by default, it adds a macro
@polly
which can be used to explicitly annotate functions that should be optimized via Polly. The@polly
macro relies on the:meta
mechanism to instruct Julia's LLVM code-generator whether to apply Polly to a particular function or not. Since Polly is simply a collection of LLVM passes this involves to selectively "turn on" these passes for the respective function. Therefore, inemit_function
, where the lowering to LLVM IR happens, I check whether the given function is marked with the special:meta
-symbol:polly
and accordingly mark the generated LLVM function with an attributePollySkipFnAttr
(this attribute is already provided by Polly and allows to tell Polly to skip its optimizations for a certain function).I do not know how I can "automatically" test whether Polly is executed for a certain function or not, so for now I had to manually verify that Polly is invoked for a function when
@polly
is present.Since these changes also regard the build process of Julia I added a build option
USE_POLLY
which can be used to enable/disable these features (which is0
by default to ensure the standard build is not influenced). WhenUSE_POLLY
is set to1
, Julia will be linked against Polly's static librarieslibPolly.a
andlibPollyISL.a
. For now, the build expects these libraries in thelib
directory of the used LLVM installation, which basically means that LLVM has to be built manually together with Polly (like described here). So to successfully build Julia to enable these features, it would be necessary to have a Make.user that contains the following configurations, where YOUR_LLVM_BUILD_DIR points to the according LLVM installation containing Polly:So to make the build a little more "user-friendly" it would be nice to have Polly be downloaded and built automatically which I'll try to solve next. Basically, this will require changing
deps/Makefile
to download Polly from its svn/git repository.Please feel free to post your thoughts and suggestions on this.