[llvm] r320413 - [cmake] Pass TARGETS_TO_BUILD through to host tools build

[Don Hinton via llvm-commits]

Hi Alex:

On Tue, Dec 12, 2017 at 2:04 AM, Alex Bradbury via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On 11 December 2017 at 19:53, Justin Bogner via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: bogner
> > Date: Mon Dec 11 11:53:23 2017
> > New Revision: 320413
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=320413&view=rev
> > Log:
> > [cmake] Pass TARGETS_TO_BUILD through to host tools build
> >
> > In r319620, the host build was changed to use Native for
> > TARGETS_TO_BUILD because passing semicolons through add_custom_command
> > is surprisingly difficult. However, Native really doesn't make any
> > sense here, and it only works because we don't technically do any
> > codegen in the host tools so pretty well anything will "work".
> >
> > The problem here is that passing something other than the correct
> > value is very fragile - as evidence note how the llvm-config in the
> > host tools acts differently than the target one now, and misreports
> > the targets to build. Similarly, if there is any logic conditional on
> > the targets in tablegen (now or in the future), it will do the wrong
> > thing.
> >
> > To fix this, we need to escape the semicolons in the targets string
> > and pass it through to the child cmake invocation.
>
> Hi Justin. This breaks builds which set targets through
> LLVM_EXPERIMENTAL_TARGETS_TO_BUILD (in my case, the RISCV backend when
> building with -DLLVM_OPTIMIZED_TABLEGEN=True). I'm not familiar enough
> with the CMake internals and the problem this patch fixes to suggest a
> minimal fix. Part of the problem is that at this point
> LLVM_EXPERIMENTAL_TARGETS_TO_BUILD has already been merged in to
> LLVM_TARGETS_TO_BUILD. Is it desirable to set
> LLVM_EXPERIMENTAL_TARGETS_TO_BUILD for the host build?
>

Yes, for this to work, we'd need to pass LLVM_EXPERIMENTAL_TARGETS_TO_BUILD
as well.

However, since Native is the only target actually used, I'd lean toward
reverting this change and perhaps adding a comment.

hth...
don


>
>
> Excerpt from failed configure:
>
> [1/1] Configuring NATIVE LLVM...
> FAILED: NATIVE/CMakeCache.txt
> cd /local/scratch/asb58/llvm-repos/llvm/build_for_upstream/NATIVE &&
> /local/scratch/asb58/cmake-3.5.2-Linux-x86_64/bin/cmake -G Ninja
> -DCMAKE_C_COMPILER=/usr/bin/clang-3.8
> -DCMAKE_CXX_COMPILER=/usr/bin/clang++-3.8
> /local/scratch/asb58/llvm-repos/llvm
> -DLLVM_TARGET_IS_CROSSCOMPILE_HOST=TRUE
> -DLLVM_TARGETS_TO_BUILD="AArch64;AMDGPU;ARM;BPF;Hexagon;Lanai;Mips;MSP430;
> NVPTX;PowerPC;Sparc;SystemZ;X86;XCore;RISCV"
> -DCMAKE_BUILD_TYPE=Release
> -- Native target architecture is X86
> -- Threads enabled.
> -- Doxygen disabled.
> -- Go bindings enabled.
> -- Could NOT find OCaml (missing:  OCAMLFIND OCAML_VERSION
> OCAML_STDLIB_PATH)
> -- Could NOT find OCaml (missing:  OCAMLFIND OCAML_VERSION
> OCAML_STDLIB_PATH)
> -- OCaml bindings disabled.
> -- Could NOT find Python module pygments
> -- Could NOT find Python module yaml
> -- LLVM host triple: x86_64-unknown-linux-gnu
> -- LLVM default target triple: x86_64-unknown-linux-gnu
> -- Building with -fPIC
> -- Constructing LLVMBuild project information
> CMake Error at CMakeLists.txt:682 (message):
>   The target `RISCV' does not exist.
>
>       It should be one of
>
>
>   AArch64;AMDGPU;ARM;BPF;Hexagon;Lanai;Mips;MSP430;
> NVPTX;PowerPC;Sparc;SystemZ;X86;XCore
>
>
> -- Configuring incomplete, errors occurred!
> See also "/local/scratch/asb58/llvm-repos/llvm/build_for_upstream/
> NATIVE/CMakeFiles/CMakeOutput.log".
> See also "/local/scratch/asb58/llvm-repos/llvm/build_for_upstream/
> NATIVE/CMakeFiles/CMakeError.log".
> ninja: build stopped: subcommand failed.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171212/4b5d3260/attachment.html>