[PATCH] D157844: [llvm][CMake] Improve error message for unknown or experimental targets

[David Spickett via Phabricator via llvm-commits]

DavidSpickett created this revision.
Herald added subscribers: sstefan1, ekilmer, luismarques, steven.zhang, pengfei, s.egerton, PkmX, atanasyan, sunfish, simoncook, fedor.sergeev, kristof.beyls, arichardson, tpr, sdardis, dylanmckay, jyknight, dschuff.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added subscribers: llvm-commits, wangpc, aheejin.
Herald added a project: LLVM.

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.
  
    Default 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157844

Files:
  llvm/CMakeLists.txt


Index: llvm/CMakeLists.txt
===================================================================
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -919,15 +919,25 @@
 foreach(t ${LLVM_TARGETS_TO_BUILD})
   set( td ${LLVM_MAIN_SRC_DIR}/lib/Target/${t} )
 
-  list(FIND LLVM_ALL_TARGETS ${t} idx)
-  list(FIND LLVM_EXPERIMENTAL_TARGETS_TO_BUILD ${t} idy)
-  # At this point, LLVMBUILDTOOL already checked all the targets passed in
-  # LLVM_TARGETS_TO_BUILD and LLVM_EXPERIMENTAL_TARGETS_TO_BUILD, so
-  # this test just makes sure that any experimental targets were passed via
+  # Make sure that any experimental targets were passed via
   # LLVM_EXPERIMENTAL_TARGETS_TO_BUILD, not LLVM_TARGETS_TO_BUILD.
-  if( idx LESS 0 AND idy LESS 0 )
-    message(FATAL_ERROR "The target `${t}' is experimental and must be passed "
-      "via LLVM_EXPERIMENTAL_TARGETS_TO_BUILD.")
+  # We allow experimental targets that are not in LLVM_ALL_EXPERIMENTAL_TARGETS,
+  # as long as they are passed via LLVM_EXPERIMENTAL_TARGETS_TO_BUILD.
+  list(FIND LLVM_ALL_TARGETS ${t} idx_all_targets)
+  list(FIND LLVM_ALL_EXPERIMENTAL_TARGETS ${t} idx_all_experimental_targets)
+  list(FIND LLVM_EXPERIMENTAL_TARGETS_TO_BUILD ${t} idx_experimental_to_build)
+
+  if( idx_all_targets LESS 0 AND idx_experimental_to_build LESS 0)
+    if( idx_all_experimental_targets GREATER_EQUAL 0)
+      message(FATAL_ERROR "The target `${t}' is experimental and must be passed "
+        "via LLVM_EXPERIMENTAL_TARGETS_TO_BUILD.")
+    else()
+      message(FATAL_ERROR "The target `${t}' is not a default target. It may be "
+        "experimental, if so it must be passed via "
+        "LLVM_EXPERIMENTAL_TARGETS_TO_BUILD.\n"
+        "Default targets: ${LLVM_ALL_TARGETS}\n"
+        "Known experimental targets: ${LLVM_ALL_EXPERIMENTAL_TARGETS}")
+    endif()
   else()
     set(LLVM_ENUM_TARGETS "${LLVM_ENUM_TARGETS}LLVM_TARGET(${t})\n")
   endif()


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157844.549832.patch
Type: text/x-patch
Size: 1919 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230814/6e182063/attachment.bin>