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.

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

Reorganize pass-pipeline for Polly #17696

Merged

Conversation

@MatthiasJReisinger
Copy link
Contributor

@MatthiasJReisinger MatthiasJReisinger commented Jul 29, 2016

We now move Polly after LoopRotate since at this point the LLVM IR is more amenable to Polly. For example, a simple gemm kernel like

@polly function gemm!(A,B,C)
    m,n = size(A)
    n,o = size(B)
    @inbounds for i=1:m, j=1:o, k=1:n
        C[i,j] += A[i,k] * B[k,j]
    end
end

could not be optimized before this change. However, this new position after LoopRotate also makes it necessary to rerun InstCombine before Polly to get rid of hindering phi instructions that are introduced by LCSSA. Additionally, we add Polly's CodePreparation and CodegenCleanUp passes. Parts of these changes have already been discussed in https://groups.google.com/forum/#!topic/polly-dev/P2RZUlKRo0I. The reason why they were held back was to see if further adaptions to the pass-pipeline will become apparent, but for now this configuration seems to be convenient.

@TobiG @vtjnash @timholy

@@ -54,7 +54,7 @@ FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --includedir)
LLVM_LIBS := all
ifeq ($(USE_POLLY),1)
LLVMLINK += -lPolly -lPollyISL
FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --src-root)/tools/polly/include
FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --src-root)/tools/polly/include -I$(shell $(LLVM_CONFIG_HOST) --obj-root)/tools/polly/include

This comment has been minimized.

@tobiasgrosser

tobiasgrosser Jul 29, 2016
Contributor

Why are makefile changes necessary to reorganize the pass pipeline?

This comment has been minimized.

@MatthiasJReisinger

MatthiasJReisinger Jul 29, 2016
Author Contributor

To use the CodePreparationPass in jitlayers.cpp it was necessary to include polly/LinkAllPasses.h which in turn includes polly/Config/config.h which is placed in $(shell $(LLVM_CONFIG_HOST) --obj-root)/tools/polly/include.

This comment has been minimized.

@tobiasgrosser

tobiasgrosser Jul 29, 2016
Contributor

On 07/29/2016 01:10 PM, Matthias J. Reisinger wrote:

In src/Makefile
#17696 (comment):

@@ -54,7 +54,7 @@ FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --includedir)
LLVM_LIBS := all
ifeq ($(USE_POLLY),1)
LLVMLINK += -lPolly -lPollyISL
-FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --src-root)/tools/polly/include
+FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --src-root)/tools/polly/include -I$(shell $(LLVM_CONFIG_HOST) --obj-root)/tools/polly/include

To use the CodePreparationPass in jitlayers.cpp it was necessary to
include polly/LinkAllPasses.h which in turn includes
polly/Config/config.h which is placed in $(shell $(LLVM_CONFIG_HOST)
--obj-root)/tools/polly/include.

OK.

Tobias

@ViralBShah
Copy link
Member

@ViralBShah ViralBShah commented Jul 29, 2016

Let's merge this after branching for 0.5.

polly::registerPollyPasses(*PM);
PM->add(polly::createCodegenCleanupPass());

This comment has been minimized.

@tobiasgrosser

tobiasgrosser Jul 29, 2016
Contributor

Codegen cleanup and codegen prepare passes are partially independent changes. I would suggest to add them in separate commits (may be the still pull-request though).

@MatthiasJReisinger
Copy link
Contributor Author

@MatthiasJReisinger MatthiasJReisinger commented Jul 29, 2016

@TobiG thank you for the feedback! I will address this and update the patch. @ViralBShah I agree, it's not necessary to merge this before 0.5.

@vtjnash
Copy link
Member

@vtjnash vtjnash commented Aug 1, 2016

lgtm. Since this is all behind preprocessor macros anyways, I see no particular reason it matters if this merges before the v0.5 branch point or after.

We now move Polly after LoopRotate since at this point the LLVM IR is more amenable to Polly. For example, a simple gemm kernel like

@polly function gemm!(A,B,C)
    m,n = size(A)
    n,o = size(B)
    @inbounds for i=1:m, j=1:o, k=1:n
        C[i,j] += A[i,k] * B[k,j]
    end
end

could not be optimized before this change. However, this new position after LoopRotate also makes it necessary to rerun InstCombine before Polly to get rid of hindering phi instructions that are introduced by LCSSA. Parts of these changes have already been discussed in https://groups.google.com/forum/#!topic/polly-dev/P2RZUlKRo0I. The reason why they were held back was to see if further adaptions to the pass-pipeline will become apparent, but for now this configuration seems to be convenient.
This pass performs a simple canonicalization needed by Polly.
Clean up the optimized code emitted by Polly.
@MatthiasJReisinger MatthiasJReisinger force-pushed the MatthiasJReisinger:mjr/polly_pass_reordering branch from 6d6bf3f to 9a54ed8 Aug 1, 2016
@vtjnash vtjnash merged commit f8535d7 into JuliaLang:master Aug 5, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MatthiasJReisinger MatthiasJReisinger deleted the MatthiasJReisinger:mjr/polly_pass_reordering branch Aug 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants