Page MenuHomePhabricator

Adding openmp project as a runtime project to projects/CMakeLists.txt
AbandonedPublic

Authored by jlpeyton on May 6 2015, 9:55 AM.

Details

Reviewers
chandlerc
Summary

We have just committed changes to our CMake system which makes it compatible for building as a project LLVM. The changes here simply add it as a runtime project which offers convenient CMake control variables for the user.

Chandler, I chose you as the reviewer because the CODE_OWNERS.txt file indicated you were the owner of the CMake build system.

Diff Detail

Repository
rL LLVM

Event Timeline

jlpeyton updated this revision to Diff 25060.May 6 2015, 9:55 AM
jlpeyton retitled this revision from to Adding openmp project as a runtime project to projects/CMakeLists.txt.
jlpeyton updated this object.
jlpeyton edited the test plan for this revision. (Show Details)
jlpeyton added a reviewer: chandlerc.
jlpeyton set the repository for this revision to rL LLVM.
jlpeyton added subscribers: AndreyChurbanov, jhowarth, Unknown Object (MLST), Unknown Object (MLST).
chandlerc edited edge metadata.May 12 2015, 4:03 PM

Cool!!!

I want to patch this locally and try it out, will review shortly.

So, looking at this, the openmp CMake isn't ready for prime time yet. It is using really weird conventions for variable naming (stuff like 'os', etc).

I think you'll need to make the openmp CMake look much more like libc++'s? That one has received some good attention recently and seems to integrate cleanly.

I don't think we can go forward here until that CMake is cleaned up.

chandlerc requested changes to this revision.May 12 2015, 5:05 PM
chandlerc edited edge metadata.
This revision now requires changes to proceed.May 12 2015, 5:05 PM
emaste added a subscriber: emaste.May 15 2015, 8:44 AM

It is using really weird conventions for variable naming (stuff like 'os', etc).

Yeah, I had a look at it for building on FreeBSD; it "just works" if I use CMake defaults, presumably because it sets os=lin which actually means "UNIX-like other than Apple." Later on there is a FREEBSD variable, but it doesn't seem to be used. I was going to take a look at fixing the FreeBSD cases, but will hold off while more fundamental rework is done.

jlpeyton abandoned this revision.May 15 2017, 11:46 AM