Update with experimental support for LLVM 6.0.0 #2595

chalcolith · 2018-03-11T02:26:18Z

This PR includes build system and code changes to support LLVM 6.0.0.

SeanTAllen · 2018-03-11T03:37:31Z

@kulibali does this work without the workarounds in llvm 4 and 5 support?

SeanTAllen · 2018-03-11T03:38:17Z

@kulibali do you know how to add support for CI testing of LLVM 6?

chalcolith · 2018-03-11T19:08:25Z

I haven't done anything on Linux yet, so I haven't tested the workaround or CI for Linux yet. The single AppVeyor failure seems spurious.

I'll look at Linux, including CI, today.

chalcolith · 2018-03-11T21:38:51Z

Ok, I have copied the CI configuration to make 6.0.0 happen; if I understand it correctly, someone will have to make a container for LLVM 6? Someone who knows Docker much better than me, ideally.

One caveat: LLVM seems to not have generated a binary tarball for Ubuntu 16.04: http://releases.llvm.org/6.0.0/

SeanTAllen · 2018-03-11T22:20:12Z

@kulibali i think either @jemc or i could create the container for you. and you could update the circleci config to test with 6 as well. Using Ubuntu 16.04 isnt a requirement for setting up the CI. If you check, you'll see we use different uses of Ubuntu for the various LLVM versions.

https://github.com/ponylang/ponyc/tree/master/.ci-dockerfiles

SeanTAllen · 2018-03-11T22:28:42Z

.travis_script.bash

+
+      export CC1=clang-6.0
+      export CXX1=clang++-6.0
+      #echo "Running LLVM 5.0 config=debug build..."


you can ditch these commented out lines

chalcolith · 2018-03-17T20:19:32Z

Hmm, I thought I had made and pushed a Docker image for LLVM 6; it seems I didn't do it right.

jemc · 2018-03-22T04:38:50Z

It looks like the docker image you built may be broken with an owner id issue, as noted by CircleCI.

I'm going to try building it myself to see if it resolves the issue.

jemc · 2018-04-03T14:58:06Z

The image also was broken when I built it.

I figured out last night that the LLVM archive seems to have been built with a "high" UID/GID, and when extracting tar archives as the root user, tar will keep the original UID/GID of the file. You can apparently override this with --no-same-owner, so I'm going to try that.

jemc · 2018-04-04T16:32:59Z

@kulibali - I was able to create (last night) and upload (this morning) a working image, using the following Dockerfile:

FROM ubuntu:14.04

ENV LLVM_VERSION 6.0.0

RUN apt-get update \
 && apt-get install -y \
  apt-transport-https \
  build-essential \
  g++ \
  git \
  libncurses5-dev \
  libssl-dev \
  make \
  wget \
  xz-utils \
  zlib1g-dev \
 && cd /tmp \
 && wget ftp://ftp.csx.cam.ac.uk/pub/software/programming/pcre/pcre2-10.21.tar.bz2 \
 && tar xjvf pcre2-10.21.tar.bz2 \
 && cd pcre2-10.21 \
 && ./configure --prefix=/usr \
 && make \
 && sudo make install \
 && cd / \
 && rm -rf /tmp/pcre* \
 && wget -O - http://releases.llvm.org/${LLVM_VERSION}/clang+llvm-${LLVM_VERSION}-x86_64-linux-gnu-ubuntu-14.04.tar.xz \
 | tar xJf - --no-same-owner --strip-components 1 -C /usr/local/ clang+llvm-${LLVM_VERSION}-x86_64-linux-gnu-ubuntu-14.04

So I think Linux CI is passing now.

The difference was using --no-same-owner in the tar extraction.

jemc · 2018-04-04T16:35:37Z

.travis_script.bash

      echo "Running LLVM 5.0 config=release build..."
      export config=release
      ponyc-test

      make clean
      brew uninstall llvm@5
+
+      # 6.0.x
+      # There is no llvm@6 package right now, so this will break once LLVM 7


I think this is no longer true? https://github.com/Homebrew/homebrew-core/blob/cd015553ae9c25389f4e35f8a9a9aef0ac2cc733/Aliases/llvm%406

Is this still true?

jemc · 2018-04-04T16:54:04Z

src/libponyc/codegen/genopt.cc

          // hoisting loads (see #2303, #2061, #1592).
-          // TODO: figure out the real reason LLVM 4 and 5 produce bad code 
+          // TODO: figure out the real reason LLVM 4 and 5 produce bad code


Have we figured out whether LLVM 6 also produces this bad code?

Yes, there are still problems with LLVM 6 on Linux, but different from 4 & 5. @Praetonus was going to look at it again in his copious free time.

I turned on the workaround again for the time being, which is why the CI tests are passing.

jemc · 2018-04-04T17:01:22Z

It looks like this just needs some OSX attention now.

jemc · 2018-04-04T20:11:21Z

Here's the error from the Travis OSX build:

src/libponyc/codegen/host.cc:42:45: error: no member named 'JITDefault' in namespace 'llvm::CodeModel'
  CodeModel::Model model = jit ? CodeModel::JITDefault : CodeModel::Default;
                                 ~~~~~~~~~~~^

src/libponyc/codegen/host.cc:42:69: error: no member named 'Default' in namespace 'llvm::CodeModel'
  CodeModel::Model model = jit ? CodeModel::JITDefault : CodeModel::Default;
                                                         ~~~~~~~~~~~^

chalcolith · 2018-04-04T20:44:02Z

I see a warning for LLVM 5 in the build log, so I think that LLVM 5 is getting installed instead of 6: Makefile:266: WARNING: LLVM 5 support is experimental and may result in decreased performance or crashes

SeanTAllen · 2018-04-04T21:07:01Z

Yes. We do all the builds on the same OSX instances. We install the different versions one at a time, testing each as we go.

…

On Wed, Apr 4, 2018, 4:44 PM Gordon Tisher ***@***.***> wrote: I see a warning for LLVM 5 in the build log, so I think that LLVM 5 is getting installed instead of 6: Makefile:266: WARNING: LLVM 5 support is experimental and may result in decreased performance or crashes — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2595 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAL0PH5lyOfgg5Nl5-itUFKQmZEVVjm_ks5tlTCTgaJpZM4Slh57> .

winksaville · 2018-04-04T22:34:41Z

I also just ran into the JITDefault problem that @jemc found. I looked at the llvm sources and found JITDefault was deleted last year with this change hopefully that helps.

jemc · 2018-04-04T22:49:59Z

Thanks, @winksaville!

So, based on that change, I think what's needed should be something like this:

#if PONY_LLVM < 600
  CodeModel::Model model = jit ? CodeModel::JITDefault : CodeModel::Default;
  TargetMachine* m = t->createTargetMachine(opt->triple, opt->cpu,
    opt->features, options, reloc, model, opt_level);
#else
  CodeModel::Model model = jit ? CodeModel::Large : CodeModel::Small;
  TargetMachine* m = t->createTargetMachine(opt->triple, opt->cpu,
    opt->features, options, reloc, model, opt_level, jit);
#end

jemc · 2018-04-04T22:50:23Z

\cc @Praetonus for confirmation on the small/large choice I made there with respect to the JIT...

chalcolith · 2018-04-04T22:58:52Z

That code is in master; my branch is a bit behind. I will rebase it, but why is travis building master and not my branch?

chalcolith · 2018-04-04T23:07:43Z

OK, I have:

rebased this branch on master and fixed the compile per @jemc's comment
updated the LLVM 6 dockerfile with @jemc's changes
added build system warnings for LLVM 6

Praetonus · 2018-04-05T00:27:51Z

The change suggested by @jemc looks correct to me, but in case the default code models change in future LLVM versions, I think we should rather do something like this:

TargetMachine* m = t->createTargetMachine(opt->triple, opt->cpu,
  opt->features, options, reloc, llvm::None, opt_level, jit);

With a None argument to the Optional<CodeModel> parameter, the LLVM internals will determine the code model to use based on the value of jit.

This change adds experimental support for LLVM 6.0.0 and removes support for 4.0.1. The change still includes the workaround in the `Heap2Stack` optimization pass that can cause performance degradation (see ponylang#2371).

chalcolith added the do not merge This PR should not be merged at this time label Mar 11, 2018

SeanTAllen reviewed Mar 11, 2018

View reviewed changes

jemc reviewed Apr 4, 2018

View reviewed changes

chalcolith force-pushed the llvm600 branch from 1012fd1 to 1f17055 Compare April 4, 2018 23:06

chalcolith and others added 6 commits April 5, 2018 08:09

Updates for LLVM 6.0.0 on Windows

ebfac97

Update AppVeyor configuration to include LLVM 6.0.0 builds

ec54837

Linux updates for LLVM 6.0.0

4c8dae6

Remove heap-to-stack workaround for LLVM 6.0.0

1743778

CI configuration for Linux and MacOS

f925fc7

Fix LLVM 6 install for OSX

730575d

chalcolith added 8 commits April 5, 2018 08:09

Fix Windows build

70beb0d

Update docker configuration for LLVM 6 container.

97beea7

Add LLVM 6 to CircleCI config.

59175ea

Add back Heap2Stack workaround for the time being.

5fb8c55

Fix typos in CircleCI config.

82f2fd6

Update for rebase

0e22fd0

Add build system warning for LLVM 6

05e4a1e

Update Dockerfile with changes from @jemc

bc4e98d

chalcolith force-pushed the llvm600 branch from 1f17055 to a6ae93a Compare April 5, 2018 15:15

chalcolith added 2 commits April 5, 2018 08:15

Update per @Praetonus's comment.

a6ae93a

Remove LLVM 4 from AppVeyor config

7a4bd1d

chalcolith added changelog - added Automatically add "Added" CHANGELOG entry on merge needs discussion during sync and removed do not merge This PR should not be merged at this time labels Apr 5, 2018

chalcolith changed the title ~~Update to support LLVM 6.0.0~~ Update with experimental support for LLVM 6.0.0 Apr 11, 2018

jemc removed the needs discussion during sync label Apr 11, 2018

jemc approved these changes Apr 11, 2018

View reviewed changes

chalcolith merged commit 48d1cb0 into ponylang:master Apr 11, 2018

ponylang-main added a commit that referenced this pull request Apr 11, 2018

Update CHANGELOG for PR #2595 [skip ci]

14e1c12

jemc deleted the llvm600 branch April 12, 2018 23:39

dipinhora pushed a commit to dipinhora/ponyc that referenced this pull request Jun 5, 2018

Update CHANGELOG for PR ponylang#2595 [skip ci]

0917699

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update with experimental support for LLVM 6.0.0 #2595

Update with experimental support for LLVM 6.0.0 #2595

chalcolith commented Mar 11, 2018

SeanTAllen commented Mar 11, 2018

SeanTAllen commented Mar 11, 2018

chalcolith commented Mar 11, 2018

chalcolith commented Mar 11, 2018

SeanTAllen commented Mar 11, 2018

SeanTAllen Mar 11, 2018

chalcolith commented Mar 17, 2018

jemc commented Mar 22, 2018

jemc commented Apr 3, 2018 •

edited

Loading

jemc commented Apr 4, 2018

jemc Apr 4, 2018 •

edited

Loading

SeanTAllen Apr 4, 2018

jemc Apr 4, 2018

chalcolith Apr 4, 2018

chalcolith Apr 4, 2018

jemc commented Apr 4, 2018

jemc commented Apr 4, 2018

chalcolith commented Apr 4, 2018

SeanTAllen commented Apr 4, 2018 via email

winksaville commented Apr 4, 2018 •

edited

Loading

jemc commented Apr 4, 2018

jemc commented Apr 4, 2018

chalcolith commented Apr 4, 2018 •

edited

Loading

chalcolith commented Apr 4, 2018 •

edited

Loading

Praetonus commented Apr 5, 2018

Update with experimental support for LLVM 6.0.0 #2595

Update with experimental support for LLVM 6.0.0 #2595

Conversation

chalcolith commented Mar 11, 2018

SeanTAllen commented Mar 11, 2018

SeanTAllen commented Mar 11, 2018

chalcolith commented Mar 11, 2018

chalcolith commented Mar 11, 2018

SeanTAllen commented Mar 11, 2018

SeanTAllen Mar 11, 2018

Choose a reason for hiding this comment

chalcolith commented Mar 17, 2018

jemc commented Mar 22, 2018

jemc commented Apr 3, 2018 • edited Loading

jemc commented Apr 4, 2018

jemc Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

SeanTAllen Apr 4, 2018

Choose a reason for hiding this comment

jemc Apr 4, 2018

Choose a reason for hiding this comment

chalcolith Apr 4, 2018

Choose a reason for hiding this comment

chalcolith Apr 4, 2018

Choose a reason for hiding this comment

jemc commented Apr 4, 2018

jemc commented Apr 4, 2018

chalcolith commented Apr 4, 2018

SeanTAllen commented Apr 4, 2018 via email

winksaville commented Apr 4, 2018 • edited Loading

jemc commented Apr 4, 2018

jemc commented Apr 4, 2018

chalcolith commented Apr 4, 2018 • edited Loading

chalcolith commented Apr 4, 2018 • edited Loading

Praetonus commented Apr 5, 2018

jemc commented Apr 3, 2018 •

edited

Loading

jemc Apr 4, 2018 •

edited

Loading

winksaville commented Apr 4, 2018 •

edited

Loading

chalcolith commented Apr 4, 2018 •

edited

Loading

chalcolith commented Apr 4, 2018 •

edited

Loading