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

Raise min LLVM version to 3.9 #2872

Merged
merged 1 commit into from
Oct 15, 2018
Merged

Conversation

kinke
Copy link
Member

@kinke kinke commented Oct 7, 2018

No description provided.

@kinke kinke force-pushed the minLLVM309 branch 3 times, most recently from 3cbde39 to 5277ce2 Compare October 7, 2018 15:54
@kinke kinke changed the title WIP: Raise min LLVM version to 3.9 Raise min LLVM version to 3.9 Oct 7, 2018
@JohanEngelen
Copy link
Member

Certainly in favor of this.

To prevent complaints later: https://forum.dlang.org/post/bmsdqayozfuxvbuwklbu@forum.dlang.org

@dnadlinger
Copy link
Member

+1, unless the forum thread brings up anything.

@kinke
Copy link
Member Author

kinke commented Oct 8, 2018

[I forgot to remove the FileCheck and llvm-profdata .cpp files, amended now.]

Copy link
Contributor

@joakim-noah joakim-noah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I didn't know how much work had gone into supporting these old LLVM versions.

@dnadlinger
Copy link
Member

Yep, the diff is nice – it's mostly leaving old code behind for a while though after tracking API changes, so not that bad.

@kinke
Copy link
Member Author

kinke commented Oct 14, 2018

It cost me time to find a workaround in #2870, and more importantly, is a PITA for the debuginfo API changes taking place between 3.7 and 3.9, which required poor Syniurge to figure out the necessary adaptations in #2826.

@kinke
Copy link
Member Author

kinke commented Oct 14, 2018

So the plan is to merge this tomorrow (giving people one week to respond to the forum thread), then adapt & merge #2870 and #2826.

@dnadlinger
Copy link
Member

(Oh yes, that's definitely a good idea. I was responding only to Joakim's comment about the amount of effort to support old versions, where in actuality, the effort is mostly spent on new versions.)

@kinke kinke merged commit 5c24f60 into ldc-developers:master Oct 15, 2018
@kinke kinke deleted the minLLVM309 branch October 15, 2018 20:32
@Geod24
Copy link
Contributor

Geod24 commented Oct 16, 2018

DMD's BuildKite is broken and the logs seem to point to this PR.
Failing build: https://buildkite.com/dlang/dmd/builds/1916#7d52edc4-50c7-49ef-9daa-50d456217d55

Logs:

-- Looking for unistd.h - found
--
  | -- GDB 7.11.1 detected
  | CMake Error at tests/CMakeLists.txt:22 (include):
  | include could not find load file:
  |  
  | /usr/lib/llvm-3.8/lib/cmake/llvm/LLVMConfig.cmake
  | Call Stack (most recent call first):
  | tests/CMakeLists.txt:27 (get_host_arch)
  |  
  |  
  | -- LDC_HOST_ARCH:
  | -- Configuring incomplete, errors occurred!
  | See also "/var/lib/buildkite-agent/builds/buildkite-agent-03-1/dlang/dmd/build/ldc-developers-ldc/bootstrap/CMakeFiles/CMakeOutput.log".
  | See also "/var/lib/buildkite-agent/builds/buildkite-agent-03-1/dlang/dmd/build/ldc-developers-ldc/bootstrap/CMakeFiles/CMakeError.log".
  | 🚨 Error: The command exited with status 1

Looks like we're trying to use 3.8. See this issue to continue the discussion.

@joakim-noah
Copy link
Contributor

Looks like the CMake config still allows 3.7/8, needs to be updated.

@kinke
Copy link
Member Author

kinke commented Oct 16, 2018

Yeah, I grepped for 30{7,8,9}, missing 3.7 here.
BuildKite still not allowing public logs access is a PITA, I don't have the credentials at hand here at work. I guess the build image is based on Ubuntu 16.04 with its LLVM 3.8.

Edit: Ah, already fixed (the DMD problem), thx Joakim.

@wilzbach
Copy link
Contributor

BuildKite still not allowing public logs access is a PITA

Yeah, but you can use:

User: dummy@dlang.io
Pw: dlangrocks

until they finally fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants