Page MenuHomePhabricator

[test-release.sh] Add Polly to the list of projects
ClosedPublic

Authored by pzheng on Jan 13 2017, 3:39 PM.

Event Timeline

pzheng updated this revision to Diff 84405.Jan 13 2017, 3:39 PM
pzheng retitled this revision from to [test-release.sh] Add Polly to the list of projects.
pzheng updated this object.
pzheng added a subscriber: llvm-commits.
hans edited edge metadata.Jan 13 2017, 3:55 PM

Should we leave it off by default? I don't think most testers have built with polly before. Maybe raise this on the release-testers list?

grosser edited edge metadata.Jan 13 2017, 10:44 PM

Hi,

this is a great change.

Regarding the default off flag, Even with this flag to on, Polly will not influence the default behavior of clang/LLVM/... So the only thing that is tested by default is that our unit tests work on all platforms and that the build system does not cause troubles.

Now, I still think that we should not force Polly on our release testers. So off sounds like the right default for this patch.

Obviously, I would very much appreciate them to test Polly, but as Hans suggested we should kindly ask this on the release testers mailing list.

Best,
Tobias

pzheng updated this revision to Diff 84590.Jan 16 2017, 1:49 PM

[test-release.sh] Add Polly to the list of projects

I agree that we should probably leave the default for Polly to be off since this is the first attempt to add it to the test release script and release testers may not be familiar with Polly. We can revisit turning it on by default in the future.

grosser accepted this revision.Jan 17 2017, 3:12 AM

This looks good to me.

After this has been committed, we should probably ask the release builders if they would be OK to also run builds with -polly enabled.

This revision is now accepted and ready to land.Jan 17 2017, 3:12 AM
hans accepted this revision.Jan 17 2017, 1:53 PM
pzheng closed this revision.Jan 17 2017, 5:14 PM

The next LLVM release is coming up, we just verified that Polly can compile all of AOSP, and Polly has been shipping in debian and ubuntu since over a year. We should consider to flip the switch now. Pengxuan, would you like to propose such a patch?

Here is the patch to enable Polly by default:
https://reviews.llvm.org/D34306