[NFC][clang] Remove superfluous header files after refactor in #132252 #132495

jthackray · 2025-03-22T00:18:11Z

Remove superfluous header files after refactor in #132252

llvmbot · 2025-03-22T00:18:42Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-backend-amdgpu

Author: Jonathan Thackray (jthackray)

Changes

Remove superfluous header files after refactor in #132252

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

11 Files Affected:

(modified) clang/lib/CodeGen/CGBuiltin.cpp (-18)
(modified) clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp (-7)
(modified) clang/lib/CodeGen/TargetBuiltins/ARM.cpp (-1)
(modified) clang/lib/CodeGen/TargetBuiltins/Hexagon.cpp (-4)
(modified) clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp (-6)
(modified) clang/lib/CodeGen/TargetBuiltins/PPC.cpp (-4)
(modified) clang/lib/CodeGen/TargetBuiltins/RISCV.cpp (-4)
(modified) clang/lib/CodeGen/TargetBuiltins/SPIR.cpp (-3)
(modified) clang/lib/CodeGen/TargetBuiltins/SystemZ.cpp (-4)
(modified) clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp (-5)
(modified) clang/lib/CodeGen/TargetBuiltins/X86.cpp (-4)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f0ba52fa41ce8..1159f6c08e9ca 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -23,36 +23,18 @@
 #include "ConstantEmitter.h"
 #include "PatternInit.h"
 #include "TargetInfo.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/AST/Attr.h"
-#include "clang/AST/Decl.h"
-#include "clang/AST/Expr.h"
 #include "clang/AST/OSLog.h"
-#include "clang/AST/OperationKinds.h"
 #include "clang/AST/StmtVisitor.h"
-#include "clang/AST/Type.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
-#include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
-#include "llvm/ADT/APFloat.h"
-#include "llvm/ADT/APInt.h"
-#include "llvm/ADT/FloatingPointMode.h"
-#include "llvm/ADT/SmallPtrSet.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/Analysis/ValueTracking.h"
-#include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsX86.h"
-#include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/MatrixBuilder.h"
-#include "llvm/IR/MemoryModelRelaxationAnnotations.h"
 #include "llvm/Support/ConvertUTF.h"
-#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
-#include <numeric>
 #include <optional>
 #include <utility>
 
diff --git a/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp b/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
index f94917c905081..76bcbe0468c66 100644
--- a/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
@@ -10,21 +10,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CGHLSLRuntime.h"
 #include "CGBuiltin.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/Analysis/ValueTracking.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsAMDGPU.h"
 #include "llvm/IR/IntrinsicsR600.h"
-#include "llvm/IR/MDBuilder.h"
-#include "llvm/IR/MatrixBuilder.h"
 #include "llvm/IR/MemoryModelRelaxationAnnotations.h"
 #include "llvm/Support/AMDGPUAddrSpace.h"
-#include "llvm/Support/ConvertUTF.h"
 
 using namespace clang;
 using namespace CodeGen;
diff --git a/clang/lib/CodeGen/TargetBuiltins/ARM.cpp b/clang/lib/CodeGen/TargetBuiltins/ARM.cpp
index fd44f4ce47b5b..63b3d17f97f85 100644
--- a/clang/lib/CodeGen/TargetBuiltins/ARM.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/ARM.cpp
@@ -15,7 +15,6 @@
 #include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsAArch64.h"
 #include "llvm/IR/IntrinsicsARM.h"
 #include "llvm/IR/IntrinsicsBPF.h"
diff --git a/clang/lib/CodeGen/TargetBuiltins/Hexagon.cpp b/clang/lib/CodeGen/TargetBuiltins/Hexagon.cpp
index 3e08477b185f7..26fe69536daa2 100644
--- a/clang/lib/CodeGen/TargetBuiltins/Hexagon.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/Hexagon.cpp
@@ -10,12 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsHexagon.h"
 
 using namespace clang;
diff --git a/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp b/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp
index ec3465737ff8a..aaac19b229905 100644
--- a/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp
@@ -10,14 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CGBuiltin.h"
-#include "CodeGenFunction.h"
-#include "CodeGenModule.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsNVPTX.h"
 
 using namespace clang;
diff --git a/clang/lib/CodeGen/TargetBuiltins/PPC.cpp b/clang/lib/CodeGen/TargetBuiltins/PPC.cpp
index 77f2d8e2f3e74..f9890285f0aab 100644
--- a/clang/lib/CodeGen/TargetBuiltins/PPC.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/PPC.cpp
@@ -10,13 +10,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CGBuiltin.h"
-#include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsPowerPC.h"
 #include "llvm/Support/ScopedPrinter.h"
 
diff --git a/clang/lib/CodeGen/TargetBuiltins/RISCV.cpp b/clang/lib/CodeGen/TargetBuiltins/RISCV.cpp
index c07bececda817..2434ae3c5d2ff 100644
--- a/clang/lib/CodeGen/TargetBuiltins/RISCV.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/RISCV.cpp
@@ -10,12 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsRISCV.h"
 #include "llvm/TargetParser/RISCVISAInfo.h"
 #include "llvm/TargetParser/RISCVTargetParser.h"
diff --git a/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp b/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp
index 844e7c8ad5d4f..acd715e1d93d6 100644
--- a/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/SPIR.cpp
@@ -10,12 +10,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CGHLSLRuntime.h"
 #include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 
 using namespace clang;
diff --git a/clang/lib/CodeGen/TargetBuiltins/SystemZ.cpp b/clang/lib/CodeGen/TargetBuiltins/SystemZ.cpp
index e13289e21fd30..a7c25b29d1dba 100644
--- a/clang/lib/CodeGen/TargetBuiltins/SystemZ.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/SystemZ.cpp
@@ -10,12 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsS390.h"
 
 using namespace clang;
diff --git a/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp b/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp
index e5b544a225098..698f43215a1be 100644
--- a/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/WebAssembly.cpp
@@ -10,13 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CGBuiltin.h"
-#include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
-#include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsWebAssembly.h"
 
 using namespace clang;
diff --git a/clang/lib/CodeGen/TargetBuiltins/X86.cpp b/clang/lib/CodeGen/TargetBuiltins/X86.cpp
index dcb0365862a1e..0466770587a42 100644
--- a/clang/lib/CodeGen/TargetBuiltins/X86.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/X86.cpp
@@ -10,13 +10,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "ABIInfo.h"
 #include "CGBuiltin.h"
-#include "CodeGenFunction.h"
-#include "TargetInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "llvm/IR/InlineAsm.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/TargetParser/X86TargetParser.h"

…32252

erichkeane

How'd you manage to find the right ones? IMO we should be using include-what-you-use on these to make sure we get it right (if you have already, disregard this).

Also, can you share before-split/after-split/after-this build time benchmarks? Does this get us back to reasonable?

jhuber6 · 2025-03-22T02:02:05Z

How'd you manage to find the right ones? IMO we should be using include-what-you-use on these to make sure we get it right (if you have already, disregard this).

Also, can you share before-split/after-split/after-this build time benchmarks? Does this get us back to reasonable?

There's a clang-tidy rule for this right? I assumed that's what this was.

erichkeane · 2025-03-22T02:07:34Z

How'd you manage to find the right ones? IMO we should be using include-what-you-use on these to make sure we get it right (if you have already, disregard this).
Also, can you share before-split/after-split/after-this build time benchmarks? Does this get us back to reasonable?

There's a clang-tidy rule for this right? I assumed that's what this was.

I'm not aware of a tidy rule that does include-file management, but if so, that is equally as good, if that is what was used. Just pushing to make sure we get this done in an automated fashion.

tmatheson-arm

LGTM. Does this actually fix the build time regression from the previous patch?

jthackray · 2025-03-25T09:52:04Z

LGTM. Does this actually fix the build time regression from the previous patch?

Apologies for the delay; I wasn't working yesterday.

Using a Graviton c8g.8xlarge instance, with a ninja build, I don't see a large build time regression. However, this is compiling in parallel, and I appreciate that official llvmbot builders may be building sequentially, which may see them take (wall-clock) longer (even if CPU used time is similar).

Build times are also quite compiler-dependent. GCC takes a lot longer than clang, especially for the previously-mentioned RISC-V file riscv_vector_builtin_cg.inc, about 3.3MB in size, that gets #included.

Using clang 18 to compile a full ninja build after a ninja clean of the entire llvm-project source tree:

Before CGBuiltin.cpp split:
  real 351.41
  user 10275.13
  sys 471.73

After CGBuiltin.cpp split:
  real 353.28
  user 10342.80
  sys 472.39

After removing superfluous header files:
  real 353.46
  user 10337.66
  sys 474.88

i.e. 2 seconds longer build time.

However, with gcc 13 compiling, using ninja a full build of llvm-project, I see:

Before CGBuiltin.cpp split:
  real 631.64
  user 11160.92
  sys 685.47

After CGBuiltin.cpp split:
  real 574.28
  user 11214.63
  sys 691.98

If only CGBuiltin.cpp and ARM.cpp is touched/updated, I see a large difference (this was what prompted me to investigate originally as I was updating ARM intrinsics; it turns out the vast majority of the time is spent compiling the RISC-V code (riscv_vector_builtin_cg.inc, ~3.3MB in size) which was previously #included in CGBuiltin.cpp):

clang 18 - before CGBuiltin.cpp split:
  real 35.47
  user 39.84
  sys 3.08

clang 18 - after split:
  real 16.00
  user 30.02
  sys 3.44

and:

gcc 13 - before CGBuiltin.cpp split:
  real 397.02 (yes, it takes 5 minutes on Graviton 4 to recompile CGBuiltin.cpp)
  user 400.35
  sys 5.54

gcc 13 - after split:
  real 49.55
  user 65.88
  sys 5.29

mshockwave · 2025-03-25T19:12:03Z

If only CGBuiltin.cpp and ARM.cpp is touched/updated, I see a large difference

This. IMHO I think quality-of-life improvements on incremental build -- especially for LLVM developers -- is already a big thing that can justify the splitting change by itself.

Regarding the marginal difference on clean builds: my guess is that compilers might spend an even longer time on another source file(s) which dominate the total compilation time.
To find out if this is true, IIRC clang's -ftime-trace can breakdown the time it spends on each input source files. It requires some data collection from the trace file though.

jthackray · 2025-03-27T12:56:20Z

How'd you manage to find the right ones? IMO we should be using include-what-you-use on these to make sure we get it right (if you have already, disregard this).

Yes, I used iwyu.

Also, can you share before-split/after-split/after-this build time benchmarks? Does this get us back to reasonable?

I've pasted my own benchmarks in an above message. tl;dr, I didn't see any overall build time increase, but incremental build time reduction using clang is cut in half from 35 seconds to 16 seconds if only modifying CGBuiltin.cpp and ARM.cpp. gcc 13 reduction was even greater, but mainly because gcc is so much slower at compilation than clang for this code.

llvmbot added clang Clang issues not falling into any other category backend:AMDGPU backend:RISC-V clang:codegen IR generation bugs: mangling, exceptions, etc. labels Mar 22, 2025

jthackray requested review from mstorsjo and jhuber6 March 22, 2025 00:19

[NFC][clang] Remove superfluous header files after refactor in llvm#1…

a55f760

…32252

jthackray force-pushed the remove-superfluous-headers branch from 7f1f78a to a55f760 Compare March 22, 2025 00:22

llvm deleted a comment from github-actions bot Mar 22, 2025

jthackray requested review from nikic and efriedma-quic March 22, 2025 00:26

jthackray mentioned this pull request Mar 22, 2025

[NFC][clang] Split clang/lib/CodeGen/CGBuiltin.cpp into target-specific files #132252

Merged

jhuber6 approved these changes Mar 22, 2025

View reviewed changes

erichkeane reviewed Mar 22, 2025

View reviewed changes

tmatheson-arm approved these changes Mar 24, 2025

View reviewed changes

jthackray merged commit a1a74c9 into llvm:main Mar 26, 2025
11 checks passed

jthackray deleted the remove-superfluous-headers branch April 8, 2025 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC][clang] Remove superfluous header files after refactor in #132252 #132495

[NFC][clang] Remove superfluous header files after refactor in #132252 #132495

Uh oh!

jthackray commented Mar 22, 2025 •

edited

Loading

Uh oh!

llvmbot commented Mar 22, 2025 •

edited

Loading

Uh oh!

erichkeane left a comment

Uh oh!

jhuber6 commented Mar 22, 2025

Uh oh!

erichkeane commented Mar 22, 2025

Uh oh!

tmatheson-arm left a comment

Uh oh!

jthackray commented Mar 25, 2025

Uh oh!

mshockwave commented Mar 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

jthackray commented Mar 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

[NFC][clang] Remove superfluous header files after refactor in #132252 #132495

[NFC][clang] Remove superfluous header files after refactor in #132252 #132495

Uh oh!

Conversation

jthackray commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

jhuber6 commented Mar 22, 2025

Uh oh!

erichkeane commented Mar 22, 2025

Uh oh!

tmatheson-arm left a comment

Choose a reason for hiding this comment

Uh oh!

jthackray commented Mar 25, 2025

Uh oh!

mshockwave commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jthackray commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jthackray commented Mar 22, 2025 •

edited

Loading

llvmbot commented Mar 22, 2025 •

edited

Loading

mshockwave commented Mar 25, 2025 •

edited

Loading

jthackray commented Mar 27, 2025 •

edited

Loading