Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

etcwilde
Copy link
Contributor

CodeGenAction.cpp.o depends on generating GenVT.inc before trying to compile it through the following header chain:

GenVT.inc
MachineValueType.h
LowLevelType.h
MachineMemOperand.h
MachineInstr.h
MachineBasicBlock.h
MachineFunctionPass.h
MachineOptimizationRemarkEmitter.h
CodeGenAction.cpp

There is a dependency edge through LLVMCodeGenTypes, but that edge is applied to the clangCodeGen link step, not the compile step of the files that make up clangCodeGen. Usually the compile and link are close enough in the build that GenVT.inc is scheduled early enough that it exists by the time we're compiling CodeGenAction.cpp.o, but on machines with high core counts, it seems to be more prevalent that the scheduling works out just right to expose the missing edge. I've only been able to reproduce this on machines with at least 64 cores (but even then it was not reliable).

Additionally, llvm-tblgen depends on GenVT.inc, so to see the missing dependency edge, one must be using a pre-built tablegen binary.

Adding the missing dependency edge to ensure that GenVT.inc is generated before trying to compile CodeGenAction.cpp.o.

Found by inspecting the dependency graph generated from Ninja with:

cmake -G 'Ninja' \
  ...
  -DLLVM_TABLEGEN=<path to native tblegen> \
  -DCLANG_TABLEGEN=<path to native clang tblgen> \
  -DLLVM_ENABLE_PROJECTS=clang \
  ...

ninja -t graph \
  tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenAction.cpp.o | dot -Tpdf > CodeGenAction.pdf

CodeGenAction.cpp.o depends on generating GenVT.inc before trying to
compile it through the following header chain:

```
GenVT.inc
MachineValueType.h
LowLevelType.h
MachineMemOperand.h
MachineInstr.h
MachineBasicBlock.h
MachineFunctionPass.h
MachineOptimizationRemarkEmitter.h
CodeGenAction.cpp
```

There is a dependency edge through LLVMCodeGenTypes, but that edge is
applied to the clangCodeGen link step, not the compile step of the files
that make up clangCodeGen. Usually the compile and link are close enough
in the build that GenVT.inc is scheduled early enough that it exists by
the time we're compiling CodeGenAction.cpp.o, but on machines with high
core counts, it seems to be more prevalent that the scheduling works out
just right to expose the missing edge. I've only been able to reproduce
this on machines with at least 64 cores (but even then it was not
reliable).

Additionally, llvm-tblgen depends on GenVT.inc, so to see the missing
dependency edge, one must be using a pre-built tablegen binary.

Adding the missing dependency edge to ensure that GenVT.inc is generated
before trying to compile CodeGenAction.cpp.o.

Found by inspecting the dependency graph generated from Ninja with:

```sh
cmake -G 'Ninja' \
  ...
  -DLLVM_TABLEGEN=<path to native tblegen> \
  -DCLANG_TABLEGEN=<path to native clang tblgen> \
  -DLLVM_ENABLE_PROJECTS=clang \
  ...

ninja -t graph \
  tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenAction.cpp.o | dot -Tpdf > CodeGenAction.pdf
```
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen labels Sep 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Evan Wilde (etcwilde)

Changes

CodeGenAction.cpp.o depends on generating GenVT.inc before trying to compile it through the following header chain:

GenVT.inc
MachineValueType.h
LowLevelType.h
MachineMemOperand.h
MachineInstr.h
MachineBasicBlock.h
MachineFunctionPass.h
MachineOptimizationRemarkEmitter.h
CodeGenAction.cpp

There is a dependency edge through LLVMCodeGenTypes, but that edge is applied to the clangCodeGen link step, not the compile step of the files that make up clangCodeGen. Usually the compile and link are close enough in the build that GenVT.inc is scheduled early enough that it exists by the time we're compiling CodeGenAction.cpp.o, but on machines with high core counts, it seems to be more prevalent that the scheduling works out just right to expose the missing edge. I've only been able to reproduce this on machines with at least 64 cores (but even then it was not reliable).

Additionally, llvm-tblgen depends on GenVT.inc, so to see the missing dependency edge, one must be using a pre-built tablegen binary.

Adding the missing dependency edge to ensure that GenVT.inc is generated before trying to compile CodeGenAction.cpp.o.

Found by inspecting the dependency graph generated from Ninja with:

cmake -G 'Ninja' \
  ...
  -DLLVM_TABLEGEN=&lt;path to native tblegen&gt; \
  -DCLANG_TABLEGEN=&lt;path to native clang tblgen&gt; \
  -DLLVM_ENABLE_PROJECTS=clang \
  ...

ninja -t graph \
  tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenAction.cpp.o | dot -Tpdf &gt; CodeGenAction.pdf

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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CMakeLists.txt (+1)
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index aa0c871c5352a8..868ec847b9634b 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -144,6 +144,7 @@ add_clang_library(clangCodeGen
   VarBypassDetector.cpp
 
   DEPENDS
+  vt_gen
   intrinsics_gen
   ClangDriverOptions
   # These generated headers are included transitively.

@etcwilde etcwilde merged commit c3fe727 into llvm:main Sep 19, 2024
11 checks passed
@etcwilde etcwilde deleted the ewilde/GenVT_deps_edge branch September 19, 2024 21:14
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
CodeGenAction.cpp.o depends on generating GenVT.inc before trying to
compile it through the following header chain:

```
GenVT.inc
MachineValueType.h
LowLevelType.h
MachineMemOperand.h
MachineInstr.h
MachineBasicBlock.h
MachineFunctionPass.h
MachineOptimizationRemarkEmitter.h
CodeGenAction.cpp
```

There is a dependency edge through LLVMCodeGenTypes, but that edge is
applied to the clangCodeGen link step, not the compile step of the files
that make up clangCodeGen. Usually the compile and link are close enough
in the build that GenVT.inc is scheduled early enough that it exists by
the time we're compiling CodeGenAction.cpp.o, but on machines with high
core counts, it seems to be more prevalent that the scheduling works out
just right to expose the missing edge. I've only been able to reproduce
this on machines with at least 64 cores (but even then it was not
reliable).

Additionally, llvm-tblgen depends on GenVT.inc, so to see the missing
dependency edge, one must be using a pre-built tablegen binary.

Adding the missing dependency edge to ensure that GenVT.inc is generated
before trying to compile CodeGenAction.cpp.o.

Found by inspecting the dependency graph generated from Ninja with:

```sh
cmake -G 'Ninja' \
  ...
  -DLLVM_TABLEGEN=<path to native tblegen> \
  -DCLANG_TABLEGEN=<path to native clang tblgen> \
  -DLLVM_ENABLE_PROJECTS=clang \
  ...

ninja -t graph \
  tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenAction.cpp.o | dot -Tpdf > CodeGenAction.pdf
```
@mgorny
Copy link
Member

mgorny commented Sep 24, 2024

This broke standalone clang builds:

-- Configuring done (6.4s)
CMake Error at /usr/lib/llvm/20/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 /usr/lib/llvm/20/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)

Since vt_gen target is part of LLVM, it's not available in standalone clang build. I guess the dep should be made conditional to CLANG_BUILT_STANDALONE.

@jroelofs
Copy link
Contributor

@mgorny #109601

porglezomp pushed a commit to swiftlang/llvm-project that referenced this pull request Oct 3, 2024
CodeGenAction.cpp.o depends on generating GenVT.inc before trying to
compile it through the following header chain:

```
GenVT.inc
MachineValueType.h
LowLevelType.h
MachineMemOperand.h
MachineInstr.h
MachineBasicBlock.h
MachineFunctionPass.h
MachineOptimizationRemarkEmitter.h
CodeGenAction.cpp
```

There is a dependency edge through LLVMCodeGenTypes, but that edge is
applied to the clangCodeGen link step, not the compile step of the files
that make up clangCodeGen. Usually the compile and link are close enough
in the build that GenVT.inc is scheduled early enough that it exists by
the time we're compiling CodeGenAction.cpp.o, but on machines with high
core counts, it seems to be more prevalent that the scheduling works out
just right to expose the missing edge. I've only been able to reproduce
this on machines with at least 64 cores (but even then it was not
reliable).

Additionally, llvm-tblgen depends on GenVT.inc, so to see the missing
dependency edge, one must be using a pre-built tablegen binary.

Adding the missing dependency edge to ensure that GenVT.inc is generated
before trying to compile CodeGenAction.cpp.o.

Found by inspecting the dependency graph generated from Ninja with:

```sh
cmake -G 'Ninja' \
  ...
  -DLLVM_TABLEGEN=<path to native tblegen> \
  -DCLANG_TABLEGEN=<path to native clang tblgen> \
  -DLLVM_ENABLE_PROJECTS=clang \
  ...

ninja -t graph \
  tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CodeGenAction.cpp.o | dot -Tpdf > CodeGenAction.pdf
```

(cherry picked from commit c3fe727)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants