[Clang] Replace vt_gen with LLVMCodeGenTypes #109601

RossComputerGuy · 2024-09-23T04:05:41Z

Fixes a build failure with Clang being built from standalone sources (environments like Nix).

Exact error:

CMake Error at /nix/store/h9yw8mg03z3dz6rgcjr7gbzkjysqc2sj-llvm-20.0.0-unstable-2024-09-22-dev/lib/cmake/llvm/AddLLVM.cmake:587 (add_dependencies):
  The dependency target "vt_gen" of target "obj.clangCodeGen" does not exist.
Call Stack (most recent call first):
  cmake/modules/AddClang.cmake:109 (llvm_add_library)
  lib/CodeGen/CMakeLists.txt:57 (add_clang_library)


CMake Error at /nix/store/h9yw8mg03z3dz6rgcjr7gbzkjysqc2sj-llvm-20.0.0-unstable-2024-09-22-dev/lib/cmake/llvm/AddLLVM.cmake:807 (add_dependencies):
  The dependency target "vt_gen" of target "clangCodeGen" does not exist.
Call Stack (most recent call first):
  cmake/modules/AddClang.cmake:109 (llvm_add_library)
  lib/CodeGen/CMakeLists.txt:57 (add_clang_library)

This fix can be reproduced via the following script (just set out to some remote source and monorepoSrc to your LLVM source dir):

mkdir -p "$out"
cp -r ${monorepoSrc}/cmake "$out"
cp -r ${monorepoSrc}/${pname} "$out"
cp -r ${monorepoSrc}/clang-tools-extra "$out"

mkdir -p "$out/llvm/include/llvm/CodeGen"
cp -r ${monorepoSrc}/llvm/include/llvm/CodeGen/CMakeLists.txt "$out/llvm/include/llvm/CodeGen/"

mkdir -p "$out/clang/include/llvm"
cp -r ${monorepoSrc}/llvm/include/llvm/CodeGen $out/clang/include/llvm/CodeGen

llvmbot · 2024-09-23T04:06:11Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Tristan Ross (RossComputerGuy)

Changes

Fixes a build failure with Clang being built from standalone sources (environments like Nix).

Exact error:

CMake Error at /nix/store/h9yw8mg03z3dz6rgcjr7gbzkjysqc2sj-llvm-20.0.0-unstable-2024-09-22-dev/lib/cmake/llvm/AddLLVM.cmake:587 (add_dependencies):
  The dependency target "vt_gen" of target "obj.clangCodeGen" does not exist.
Call Stack (most recent call first):
  cmake/modules/AddClang.cmake:109 (llvm_add_library)
  lib/CodeGen/CMakeLists.txt:57 (add_clang_library)


CMake Error at /nix/store/h9yw8mg03z3dz6rgcjr7gbzkjysqc2sj-llvm-20.0.0-unstable-2024-09-22-dev/lib/cmake/llvm/AddLLVM.cmake:807 (add_dependencies):
  The dependency target "vt_gen" of target "clangCodeGen" does not exist.
Call Stack (most recent call first):
  cmake/modules/AddClang.cmake:109 (llvm_add_library)
  lib/CodeGen/CMakeLists.txt:57 (add_clang_library)

This fix can be reproduced via the following script (just set out to some remote source and monorepoSrc to your LLVM source dir):

mkdir -p "$out"
cp -r ${monorepoSrc}/cmake "$out"
cp -r ${monorepoSrc}/${pname} "$out"
cp -r ${monorepoSrc}/clang-tools-extra "$out"

mkdir -p "$out/llvm/include/llvm/CodeGen"
cp -r ${monorepoSrc}/llvm/include/llvm/CodeGen/CMakeLists.txt "$out/llvm/include/llvm/CodeGen/"

mkdir -p "$out/clang/include/llvm"
cp -r ${monorepoSrc}/llvm/include/llvm/CodeGen $out/clang/include/llvm/CodeGen

Full diff: https://github.com/llvm/llvm-project/pull/109601.diff

1 Files Affected:

(modified) clang/lib/CodeGen/CMakeLists.txt (+4)

diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 868ec847b9634b..2b1be3f4c3035d 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -1,3 +1,7 @@
+if(EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen)
+  add_subdirectory(${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen llvm/lib/CodeGen)
+endif()
+
 set(LLVM_LINK_COMPONENTS
   AggressiveInstCombine
   Analysis

compnerd · 2024-09-23T18:27:45Z

clang/lib/CodeGen/CMakeLists.txt

@@ -1,3 +1,7 @@
+if(EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen AND CLANG_BUILT_STANDALONE)


I think that I would rather have the cases mirrored here - the path should always exist in general, it is only the standalone case that is special.

Suggested change

if(EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen AND CLANG_BUILT_STANDALONE)

if(CLANG_BUILT_STANDALONE AND EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen)

Ok, will do.

compnerd · 2024-09-23T18:28:16Z

clang/lib/CodeGen/CMakeLists.txt

@@ -1,3 +1,7 @@
+if(EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen AND CLANG_BUILT_STANDALONE)
+  add_subdirectory(${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen llvm/lib/CodeGen)
+endif()


IIRC, even in the standalone case, you need to identify the LLVM build and pass that in via LLVM_DIR, why is that not sufficient for finding this include path?

I'm not sure, are you meaning to use LLVM_DIR instead of LLVM_MAIN_SRC_DIR?

No, LLVM_DIR is passed to the CMake configure step to locate the build of LLVM. That should wire up the include path that should be needed either through a dependency on LLVMCodeGen or through a global include search path.

I'm not exactly sure what you're meaning.

The issue is that the vt_gen target is undefined. This is expected. The target should be defined by LLVM, which is a dependency. You should be wiring up the targets from LLVM into the clang build, which would provide the dependency and avoid this issue. The target definition is setup by find_package(LLVM) which is why you need to specify -D LLVM_DIR=... to the CMake invocation when configuring clang standalone. The normal build is unified and the target is defined when clang is configured.

Was that on a clean configure + build?

Yes, Nix always performs a clean configure and build.

~~https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CMakeLists.txt#L28-L38 I suspect that you just need to extend that with vt_gen.~~ Nope, there is an explicit dependency on vt_gen.

Can you get away with the addition of LLVM_LINK_COMPONENTS LLVMCodeGenTypes to the ClangCodeGen target and remove the vt_gen dependency?

Looks like CMake accepted that change, let's see if it actually compiles.

Sweet, that worked.

jroelofs · 2024-09-23T18:38:28Z

@compnerd @etcwilde WDYT about conditionalizing the new dependency with an if (TARGET vt_gen) instead of doing the add_subdirectory() dance?

compnerd · 2024-09-24T05:00:01Z

@compnerd @etcwilde WDYT about conditionalizing the new dependency with an if (TARGET vt_gen) instead of doing the add_subdirectory() dance?

Ugh, I misread the change as include_directories. The add_subdirectory is definitely odd and I'm not sure if that is a proper solution.

RossComputerGuy · 2024-09-24T05:03:40Z

The add_subdirectory is definitely odd and I'm not sure if that is a proper solution.

I'm not familiar with how LLVM CodeGen works and I am a bit new to LLVM but I looked at clang/CMakeLists.txt and so I basically just copied this add_directories.

mgorny

Thanks! This fixes the problem with standalone builds for me.

etcwilde · 2024-09-24T14:56:13Z

This drops the dependency edge between the compile step and generating GenVT.inc though. There was already a (indirect) dependency between the link step of clangCodeGen and generating GetnVT.inc.
Perhaps LLVM should export vt_gen like it does intrinsics_gen so that standalone builds can pick it up?

compnerd · 2024-09-24T14:56:21Z

clang/lib/CodeGen/CMakeLists.txt

@@ -31,6 +31,7 @@ set(LLVM_LINK_COMPONENTS
  Target
  TargetParser
  TransformUtils
+  LLVMCodeGenTypes


These are lexicographically sorted, please maintain that invariant.

Oh, didn't realize that.

compnerd · 2024-09-24T14:57:30Z

clang/lib/CodeGen/CMakeLists.txt

@@ -31,6 +31,7 @@ set(LLVM_LINK_COMPONENTS
  Target
  TargetParser
  TransformUtils
+  CodeGenTypes


This is duplicated (L6).

RossComputerGuy · 2024-09-24T14:57:43Z

You're welcome, though it looks like CI is borked.

ld.lld: error: unable to find library -lLLVMLLVMCodeGenTypes
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I've changed it from LLVMCodeGenTypes to CodeGenTypes so hopefully CI passes now.

etcwilde · 2024-09-24T15:24:40Z

Here, I'd missed adding vt_gen as something that one could pick up through the generated LLVM cmake config file. Can you try a standalone build with the changes here? #109817
Thanks.

RossComputerGuy · 2024-09-25T02:01:57Z

Replaced by #109817

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Sep 23, 2024

RossComputerGuy mentioned this pull request Sep 23, 2024

llvmPackages_git: 20.0.0-git-2024-09-16 -> 20.0.0-git-2024-09-22 NixOS/nixpkgs#343867

Merged

13 tasks

RossComputerGuy force-pushed the fix/clang-codegen-standalone branch from a64ba9c to cab2a5c Compare September 23, 2024 04:12

efriedma-quic requested review from etcwilde, compnerd and jroelofs September 23, 2024 17:01

compnerd reviewed Sep 23, 2024

View reviewed changes

RossComputerGuy force-pushed the fix/clang-codegen-standalone branch 2 times, most recently from 8e4e93f to 260d427 Compare September 24, 2024 06:28

RossComputerGuy changed the title ~~[Clang] Include LLVM CodeGen CMake file~~ [Clang] Replace vt_gen with LLVMCodeGenTypes Sep 24, 2024

jroelofs mentioned this pull request Sep 24, 2024

Add missing deps edge: CodeGenAction.cpp.o -> GenVT.inc #109306

Merged

mgorny approved these changes Sep 24, 2024

View reviewed changes

RossComputerGuy force-pushed the fix/clang-codegen-standalone branch from 260d427 to b97cb5a Compare September 24, 2024 14:56

compnerd reviewed Sep 24, 2024

View reviewed changes

[Clang] Replace vt_gen with LLVMCodeGenTypes

6902072

RossComputerGuy force-pushed the fix/clang-codegen-standalone branch from b97cb5a to 6902072 Compare September 24, 2024 14:59

RossComputerGuy closed this Sep 25, 2024

RossComputerGuy deleted the fix/clang-codegen-standalone branch September 25, 2024 02:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] Replace vt_gen with LLVMCodeGenTypes #109601

[Clang] Replace vt_gen with LLVMCodeGenTypes #109601

RossComputerGuy commented Sep 23, 2024

llvmbot commented Sep 23, 2024 •

edited

Loading

compnerd Sep 23, 2024

RossComputerGuy Sep 23, 2024

compnerd Sep 23, 2024

RossComputerGuy Sep 23, 2024

compnerd Sep 24, 2024

RossComputerGuy Sep 24, 2024

compnerd Sep 24, 2024

compnerd Sep 24, 2024

RossComputerGuy Sep 24, 2024

compnerd Sep 24, 2024 •

edited

Loading

RossComputerGuy Sep 24, 2024

RossComputerGuy Sep 24, 2024

jroelofs commented Sep 23, 2024

compnerd commented Sep 24, 2024

RossComputerGuy commented Sep 24, 2024

mgorny left a comment

etcwilde commented Sep 24, 2024

compnerd Sep 24, 2024

RossComputerGuy Sep 24, 2024

compnerd Sep 24, 2024

RossComputerGuy Sep 24, 2024

RossComputerGuy commented Sep 24, 2024

etcwilde commented Sep 24, 2024 •

edited

Loading

RossComputerGuy commented Sep 25, 2024

		@@ -1,3 +1,7 @@
		if(EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen AND CLANG_BUILT_STANDALONE)

	if(EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen AND CLANG_BUILT_STANDALONE)
	if(CLANG_BUILT_STANDALONE AND EXISTS ${LLVM_MAIN_SRC_DIR}/include/llvm/CodeGen)

[Clang] Replace vt_gen with LLVMCodeGenTypes #109601

[Clang] Replace vt_gen with LLVMCodeGenTypes #109601

Conversation

RossComputerGuy commented Sep 23, 2024

llvmbot commented Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compnerd Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jroelofs commented Sep 23, 2024

compnerd commented Sep 24, 2024

RossComputerGuy commented Sep 24, 2024

mgorny left a comment

Choose a reason for hiding this comment

etcwilde commented Sep 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossComputerGuy commented Sep 24, 2024

etcwilde commented Sep 24, 2024 • edited Loading

RossComputerGuy commented Sep 25, 2024

llvmbot commented Sep 23, 2024 •

edited

Loading

compnerd Sep 24, 2024 •

edited

Loading

etcwilde commented Sep 24, 2024 •

edited

Loading