llvmGen: move to LLVM 3.6 exclusively
ClosedPublic

Authored by bgamari on Nov 28 2014, 10:51 AM.

Details

Summary

Rework llvmGen to use LLVM 3.6 exclusively. The plans for the 7.12 release are to ship LLVM alongside GHC in the interests of user (and developer) sanity.

Along the way, refactor TNTC support to take advantage of the new prefix data support in LLVM 3.6. This allows us to drop the section-reordering component of the LLVM mangler.

Test Plan

Validate, look at emitted code

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bgamari updated this revision to Diff 1754.Nov 28 2014, 10:51 AM
bgamari retitled this revision from to RFC: Move to LLVM 3.6.
bgamari updated this object.
bgamari edited the test plan for this revision. (Show Details)
bgamari added reviewers: dterei, austin, scpmw.
bgamari updated this object.Nov 28 2014, 10:55 AM
bgamari updated this revision to Diff 1755.Nov 28 2014, 11:01 AM
  • Kill unused binding
bgamari updated this revision to Diff 1756.Nov 28 2014, 11:02 AM

Dropped first patch

bgamari updated this revision to Diff 1757.Nov 28 2014, 11:03 AM

Fix lints

its worth noting that harbor master currently doesnt do a validate using llvm backend, so we cant test this on phab yet, right?

austin edited edge metadata.Nov 28 2014, 4:17 PM

I haven't looked a the actual diff, but this seems pretty awesome to me if we ship it with 7.12 and use LLVM 3.6 as our first fixed-version of the backend. I'm also glad to see the backend using standard infrastructure provided http://reviews.llvm.org/D6454 gets accepted.

scpmw edited edge metadata.Nov 28 2014, 4:35 PM

Good initiative! This would make our life easier indeed. The real question is probably whether the LLVM change will go through? After all, from their point of view this is probably yet another extension that only GHC will ever use...

@scpmw, I'm hopeful that they will be willing to take it. Multiple LLVM developers have agreed that this looks like a reasonable interface and it seems like the prefix data facility has relatively few users, so the cost of changing these semantics isn't too great. Only time will tell, though...

The needed llvm diff has been merged.

awson added a subscriber: awson.Jan 20 2015, 2:00 AM

Could we hope TNTC refactoring using stock LLVM mechanism would automatically solve Trac #8974?

austin accepted this revision.Jan 20 2015, 4:05 PM
austin edited edge metadata.

OK, so I did a review, and this LGTM. @bgamari, since your LLVM patch is now upstream, should we go ahead and work on merging this into HEAD so it can get the biggest amount of testing possible for 7.12, and LLVM 3.6? Have you at least run the testsuite with make fast WAY="llvm optllvm" or anything yet? We should do that first.

This revision is now accepted and ready to land.Jan 20 2015, 4:05 PM

@awson, I don't believe so. I'm frankly still not sure I understand exactly what is going on in that issue.

bgamari updated this revision to Diff 2147.Jan 20 2015, 5:08 PM
bgamari edited edge metadata.
  • Kill unused binding
  • Fix lints
  • llvmGen: metadata no longer marked with metadata keyword

@austin, I just updated the Diff to reflect some changes to metadata syntax that will be in LLVM 3.6. I've been testing this as it has progressed, although I'd like to do one more round on x86_64 and ARM before we merge.

@austin, also, it may not be a terrible idea to come up with a plan on how to provide LLVM images to ensure the barrier is reasonably low for contributors to test against the LLVM backend during the 7.12 development cycle.

In D530#17808, @bgamari wrote:

@austin, also, it may not be a terrible idea to come up with a plan on how to provide LLVM images to ensure the barrier is reasonably low for contributors to test against the LLVM backend during the 7.12 development cycle.

I think this can be somewhat mitigated in the short term by doing ./validate --slow builds through Harbormaster; by doing this we'll at least make sure every patch is tested robustly against the backend through the testsuite, even if we don't have images at this exact moment for users to consume.

In D530#17839, @austin wrote:

I think this can be somewhat mitigated in the short term by doing ./validate --slow builds through Harbormaster; by doing this we'll at least make sure every patch is tested robustly against the backend through the testsuite, even if we don't have images at this exact moment for users to consume.

And I suppose if we merge we'll just use a snapshot of LLVM's 3.6 release branch for the Harbormaster builds?

bgamari added a subscriber: erikd.Jan 21 2015, 9:30 AM

@austin, for the record both the llvm and llvmopt ways pass the testsuite with no unexpected failures with LLVM r226171 on my x86_64 Linux box (as well as D624). ARM test is running. Perhaps @erikd could test on AArch64?

In D530#17847, @bgamari wrote:
In D530#17839, @austin wrote:

I think this can be somewhat mitigated in the short term by doing ./validate --slow builds through Harbormaster; by doing this we'll at least make sure every patch is tested robustly against the backend through the testsuite, even if we don't have images at this exact moment for users to consume.

And I suppose if we merge we'll just use a snapshot of LLVM's 3.6 release branch for the Harbormaster builds?

Yeah, exactly. We can re-synchronize it occasionally until the 3.6 release, to make sure it works properly with a valid release version.

Alright, works for me. I've tested on ARM with both the optllvm and llvm ways. optllvm has two failures which I'll need to look into. Otherwise things look good.

Alright then, I say we land it.

austin retitled this revision from RFC: Move to LLVM 3.6 to compiler: move to LLVM 3.6 exclusively.Jan 28 2015, 9:08 AM
austin updated this object.
austin retitled this revision from compiler: move to LLVM 3.6 exclusively to llvmGen: move to LLVM 3.6 exclusively.Feb 9 2015, 3:19 PM
austin updated this object.
austin updated the Trac tickets for this revision.
This revision was automatically updated to reflect the committed changes.