This is an archive of the discontinued LLVM Phabricator instance.

[llvm][CMake] Improve error message for unknown or experimental targets
ClosedPublic

Authored by DavidSpickett on Aug 14 2023, 2:05 AM.

Details

Summary

Previously you would get this error when passing an experimental target
via LLVM_TARGETS_TO_BUILD:

cmake ../llvm-project/llvm -DLLVM_TARGETS_TO_BUILD=M68k -DCMAKE_BU
ILD_TYPE=Release -GNinja

CMake Error at CMakeLists.txt:929 (message):
  The target `M68k' is experimental and must be passed via
  LLVM_EXPERIMENTAL_TARGETS_TO_BUILD

Since M68k is a known experimental target, this is helpful. However,
any made up target name would give you the same error.

cmake ../llvm-project/llvm -DLLVM_TARGETS_TO_BUILD=NotATarget -DCMAKE_BUILD_TYPE=Release -GNinja

CMake Error at CMakeLists.txt:929 (message):
  The target `NotATarget' is experimental and must be passed via
  LLVM_EXPERIMENTAL_TARGETS_TO_BUILD.

We know the set of default targets and in tree experimental targets,
so let's be more specific if we know that it is not an in tree experimental
target:

CMake Error at CMakeLists.txt:934 (message):
  The target `NotATarget' is not a default target.  It may be experimental,
  if so it must be passed via LLVM_EXPERIMENTAL_TARGETS_TO_BUILD.

  Core tier targets:
  AArch64;AMDGPU;ARM;AVR;BPF;Hexagon;Lanai;LoongArch;Mips;MSP430;NVPTX;PowerPC;RISCV;Sparc;SystemZ;VE;WebAssembly;X86;XCore

  Known experimental targets: ARC;CSKY;DirectX;M68k;SPIRV;Xtensa

It "may" be an experimental target because we do allow users to specify
targets that are not in LLVM_ALL_EXPERIMENTAL_TARGETS, and they will work
as long as lib/Target/<target> exists.

Maybe that could be made more strict but it would break a bunch of
forks for not much gain.

The known target names are listed to help users trying to configure
architectures with confusing naming schemes, for example Arm. Which is
variously called ARM/Arm/Armv7/AArch32 across llvm and other software.

Diff Detail

Event Timeline

DavidSpickett created this revision.Aug 14 2023, 2:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2023, 2:05 AM
DavidSpickett requested review of this revision.Aug 14 2023, 2:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2023, 2:05 AM
DavidSpickett added inline comments.
llvm/CMakeLists.txt
924

This is the only reference to LLVMBUILDTOOL in the monorepo. Perhaps it meant something once but it isn't doing anything now.

beanz added inline comments.Aug 14 2023, 6:51 AM
llvm/CMakeLists.txt
924

Yea, this is referring to the old LLVMBuild system that was used to manage the build dependency graph. It was removed, so the comment is out of date.

930

I find the CMake if(… IN_LIST …) syntax to be easier to read and follow (see: https://cmake.org/cmake/help/latest/command/if.html).

935

nit: I don’t really love the “default” target phrasing. We don’t really have a good way of distinguishing the “all but experimental” target list.

Two alternate suggestions:

  1. We could call them “known” because they are targets known to the build system.
  2. We could call them “core tier” which would borrow language from the community support policy (see: https://www.llvm.org/docs/SupportPolicy.html).

I slightly prefer option 2.

  • Use IN_LIST.
  • Refer to default backends as "core tier".
DavidSpickett marked 2 inline comments as done.Aug 14 2023, 8:24 AM
DavidSpickett added inline comments.
llvm/CMakeLists.txt
935

Agreed, let's use the pre-existing term.

DavidSpickett marked an inline comment as done.Aug 14 2023, 8:30 AM

Use the "core tier" name when printing the known targets.

beanz accepted this revision.Aug 17 2023, 6:10 AM

LGTM

This revision is now accepted and ready to land.Aug 17 2023, 6:10 AM
DavidSpickett edited the summary of this revision. (Show Details)Aug 17 2023, 6:21 AM