This is an archive of the discontinued LLVM Phabricator instance.

[OCaml] [cmake] Add CMake buildsystem for OCaml.
ClosedPublic

Authored by whitequark on Nov 3 2014, 3:17 PM.

Diff Detail

Event Timeline

whitequark updated this revision to Diff 15737.Nov 3 2014, 3:17 PM
whitequark retitled this revision from to [OCaml] [cmake] Add CMake buildsystem for OCaml..
whitequark updated this object.
whitequark edited the test plan for this revision. (Show Details)
whitequark added a reviewer: chandlerc.
whitequark set the repository for this revision to rL LLVM.
whitequark added a subscriber: Unknown Object (MLST).

This CMake buildsystem port should be functionally equivalent to the autoconf one, including:

  • Proper handling of dependencies between OCaml libraries and between OCaml and C++ libraries, including with parallel builds;
  • Embedding of equivalent linker flags in cma/cmxa libraries (not identical, as CMake shared library split LLVM harder);
  • Installation;
  • Generating documentation.

I have tested it extensively using the following matrix, which is a superset of the lit tests:

linux × shared/static builds × autoconf/cmake (for comparison) × ocaml toplevel/ocamlc/ocamlc -custom/ocamlopt.

All combinations appear to work.

rnk added a subscriber: rnk.Nov 3 2014, 4:40 PM

This looks refreshingly clean and simple.

cmake/modules/AddOCaml.cmake
8

Can you add a bit more explanatory text here about what the arguments are? It's not clear from the context of the caller what these are supposed to be.

whitequark updated this revision to Diff 15751.Nov 4 2014, 2:03 AM
whitequark set the repository for this revision to rL LLVM.
  • Add documentation in AddOCaml.
  • Build documentation for the Llvm_$BACKEND modules, too.
  • Install documentation in docs/ocaml/html rather than a Debian-specific directory, as it was earlier.
  • Remove whitespace changes.
  • Remove some accidentally committed code.
chandlerc edited edge metadata.Nov 17 2014, 8:43 AM

FYI, I'll let Reid finish the review here as he has looked at this in detail.

My only comment is that there is a confusing mixture of indentation styles here (4-space vs. 2-space, etc.). I understand that LLVM's existing CMake isn't the most consistent in this respect, but I think we're increasingly 2-space indent. I would at least try to be consistent with all the cmake bits you're adding here.

whitequark updated this revision to Diff 16400.Nov 19 2014, 2:21 PM
whitequark edited edge metadata.

Reindent to 2 spaces.

This diff should address all concerns discussed.

whitequark updated this revision to Diff 16402.Nov 19 2014, 2:23 PM

Previous upload accidentally included a few unrelated commits.

rnk accepted this revision.Dec 1 2014, 11:33 AM
rnk added a reviewer: rnk.

lgtm

Thanks!

This revision is now accepted and ready to land.Dec 1 2014, 11:33 AM