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

RFC: Download and build Polly when USE_POLLY=1 #16726

Merged
merged 1 commit into from Jun 3, 2016

Conversation

4 participants
@MatthiasJReisinger
Contributor

MatthiasJReisinger commented Jun 2, 2016

This enables the build process to automatically download and build Polly. An earlier version of this functionality has already been part of #16531, but as proposed by @tkelman I've set up this separate PR for it.

In its current state, this PR allows to enable Polly more easily - it only requires the following lines in a Make.user file and its no longer needed to download/build Polly+LLVM yourself:

USE_POLLY:=1
LLVM_VER:=svn

As you can see, it is currently required to use the svn version of LLVM for this. However, if desired I can generalize this and test whether it is possible to use older versions of Polly/LLVM as well here (so far this was not my primary focus for this).

If there's anything that's unclear or if there's something important that I missed, I'll be glad about questions/comments/advice.
@tobig @vtjnash @timholy

@tobig

This comment has been minimized.

Contributor

tobig commented Jun 2, 2016

From my perspective this looks good. I also don't think it makes any sense to support older version of Polly. They are unlikely to be impactful as of today and we will not port your changes to these older versions, such that they will continue to only work occasionally. For users it is a lot easier to just not support Polly, rather then having them to figure out that it just does not work as expected.

@Keno

This comment has been minimized.

Member

Keno commented Jun 2, 2016

This looks fine to me. I'm also fine with only supporting it on svn while we are experimenting with it.

@tkelman

This comment has been minimized.

Contributor

tkelman commented Jun 2, 2016

LGTM as-is. As long as using Polly with Julia is an experimental disabled-by-default option it doesn't need to be distribution-worthy. Just curious, how much are you needing to change upstream to make this all work?

@MatthiasJReisinger

This comment has been minimized.

Contributor

MatthiasJReisinger commented Jun 2, 2016

Thank you for the feedback so far. For now the plain process of making Polly available in Julia should be done with this. Currently I am experimenting to identify problems that Polly may have with the LLVM code that Julia emits. So besides adapting Polly itself there might also follow changes to Julia's codegen, this however is not yet really predictable.

@MatthiasJReisinger

This comment has been minimized.

Contributor

MatthiasJReisinger commented Jun 3, 2016

I apologize, this didn't answer your question @tkelman. For the next changes it will probably not be necessary to have them upstream. For #16531 and this here though I was a bit enthusiastic to get them upstream, so that others can start using Polly :)

@tkelman

This comment has been minimized.

Contributor

tkelman commented Jun 3, 2016

When I say "upstream" I generally mean changes to llvm/polly, not your PR's here.

@MatthiasJReisinger MatthiasJReisinger changed the title from WIP: Download and build Polly when USE_POLLY=1 to RFC: Download and build Polly when USE_POLLY=1 Jun 3, 2016

@MatthiasJReisinger

This comment has been minimized.

Contributor

MatthiasJReisinger commented Jun 3, 2016

Okay, this was a misunderstanding then, thank you for the clarification.

@tkelman tkelman merged commit ad367f4 into JuliaLang:master Jun 3, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment