This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Flag recursive cmake invocations for cross-compile
ClosedPublic

Authored by JosephTremoulet on Sep 7 2015, 9:52 AM.

Details

Summary

Cross-compilation uses recursive cmake invocations to build native host
tools. These recursive invocations only forward a fixed set of
variables/options, since the native environment is generally the default.
This change adds -DLLVM_TARGET_IS_CROSSCOMPILE_HOST=TRUE to the recursive
cmake invocations, so that cmake files can distinguish these recursive
invocations from top-level ones, which can explain why expected options
are unset.

LLILC will use this to avoid trying to generate its build rules in the
crosscompile native host target (where it is not needed), which would fail
if attempted because LLILC requires a cmake variable passed on the command
line, which is not forwarded in the recursive invocation.

Diff Detail

Event Timeline

JosephTremoulet retitled this revision from to [CMake] Flag recursive cmake invocations for cross-compile.
JosephTremoulet updated this object.
JosephTremoulet added reviewers: beanz, rnk.
JosephTremoulet added a subscriber: llvm-commits.

The motivation here is to enable LLVM_OPTIMIZED_TABLEGEN in LLILC, since we see a dramatic improvement in build time with it (measured on my laptop, clean debug build time drops from 4 hours to 15 minutes). So I picked reviewers based on D7349, which added LLVM_OPTIMIZED_TABLEGEN. I'm certainly open to alternative approaches, this was the cleanest I came up with short of redoing how crosscompile is handled.

beanz edited edge metadata.Sep 8 2015, 10:27 AM

I'm not opposed to having a variable like LLVM_TARGET_IS_CROSSCOMPILE_HOST, but in this patch it doesn't seem to do anything in the LLVM tree.

Since you're trying to work around a problem in LLILC, maybe it would make more sense to have LLILC not generate its build files in the host build. You could have LLILC disable itself by default unless the required variables are passed.

Since you're trying to work around a problem in LLILC, maybe it would make more sense to have LLILC not generate its build files in the host build. You could have LLILC disable itself by default unless the required variables are passed.

I thought about that, but I couldn't find a way to generate a reasonable error message for the case that a user forgets to pass the required variable to their LLILC cmake invocation -- that would look the same from the perspective of LLILC's cmakefiles as the recursive invocation here which needs to quietly skip generating LLILC build files.

beanz added a comment.Sep 8 2015, 11:11 AM

Hmmm... I'm trying to think if there is a better way to do this. I assume there is no sensible default for the required variable. I'm not familiar with the LLILC build system, can you explain how the required variable works?

I don't want to hold this up, so in lieu of a better solution I don't have any objection to this patch. I've thought about someday having the ability to build tablegen and llvm-config without bringing in all the extra targets and libraries, but I haven't come up with an actionable way to get to that.

I'm not familiar with the LLILC build system, can you explain how the required variable works?

Sure, happy to.

In brief: it tells us where to find a CoreCLR enlistment since we share some source files with them.

In long:
LLILC depends not only on LLVM, but also on the CoreCLR, and in some sense it's logically a sub-project of each. So right off the bat it's an interesting question which of those source trees it makes sense to nest under each other. We've settled on supporting both a configuration where LLILC lives in llvm/tools/llilc (with CoreCLR off somewhere else), and a configuration where all three trees are independent. In either of those locations, there's no sensible default location to put/find the CoreCLR.
A point which I just glossed over but which may be interesting is that we're actually looking for the *output* of a CoreCLR build. We're looking for source files in it, but as part of the build the relevant source files get copied over next to the output so that clients like us can pick them up (and eventually so that we can avoid having to build the CoreCLR locally and can just install a package with these sources and its binaries).
I believe a common pattern in cmake is to look for required binaries on the path (if no explicit location is given). While the source files we're looking for here aren't binaries, they are next to some binaries, so we could require those binaries be on the path to build LLILC and avoid touching these LLVM files that way. But those binaries aren't actually used as part of the build and typically we wouldn't have them on our path, so this seems awfully circuitous and I'd prefer the approach in this patch.
Another option I considered is that we could require specifying the CoreCLR location in an environment variable rather than a cmake variable. But then the spot in our cmakefiles that consult the environment variable would literally be the only use of that variable, so from a user perspective it would be confusing to need to specify this one setting as an environment variable when you can specify all the others as cmake variables.
The best analog that I can think of (to some other cmake default problem) would be how to identify where to find system headers. Correct me if I'm wrong (I'm new to cmake and haven't developed on *nix in many years), but my understanding is that

  • cmake has a considerable amount of logic for sorting that out
  • *nix systems default to some fixed paths like /user/include, which doesn't work well here since people can have their coreclr enlistment/binaries wherever they want
  • windows/MSVC requires you to set INCPATH &c environment variables, but as mentioned above environment variables don't seem like a good approach here

So that's the line of thinking that led me here.

BTW, thanks a ton for making OPTIMIZED_TABLEGEN a thing, it's a huge win here.

beanz accepted this revision.Sep 8 2015, 1:06 PM
beanz edited edge metadata.

I think the proper solution for this in the long run would be either for CoreCLR to vend CMake packaging information or for LLILC to provide the package information. There are a number of "Find*.cmake" files in the LLVM tree that provide the interfaces for CMake's find_package() APIs, and that is probably how CoreCLR should be hooked up. If it were hooked up like that you wouldn't need to pass a command line option (unless as an override).

I don't know enough about CoreCLR, or about how the CMake package finding stuff works on Windows to know how difficult it would be to make that work.

The patch as is LGTM, but longer term I'd really like to see CoreCLR working as a CMake package.

This revision is now accepted and ready to land.Sep 8 2015, 1:06 PM

Thanks for the pointer; I didn't know about find_package, but I agree it seems like the right way to do this sort of thing. We're still ironing out how these CoreCLR sources get packaged/communicated, so there isn't really any logic I could put in it today to reliably find them, but I've logged one CoreCLR issue[1] and one LLILC issue[2] to track that we'd like to make that possible and then use find_package in LLILC's cmake files, respectively.

[1] - https://github.com/dotnet/coreclr/issues/1521
[2] - https://github.com/dotnet/llilc/issues/832