Reland [Clang][Cmake] fix libtool duplicate member name warnings #133850

farzonl · 2025-04-01T04:13:46Z

~~The fix to reland was moving Targets/DirectX.cpp from clang/lib/CodeGen/Targets/CMakeLists.txt back to clang/lib/CodeGen/CMakeLists.txt.~~

~~That change means that the files in clang/lib/CodeGen/Targets/CMakeLists.txt needs to be marked PARTIAL_SOURCES_INTENDED. That address the linker missing references issue from #133776~~

As of the third commit the fix to Targets/DirectX.cpp linker issues was to move HLSLBufferLayoutBuilder.cpp to clang/lib/CodeGen/Targets/.

It fixes the circular reference issue from #133619 for all -DBUILD_SHARED_LIBS=ON builds
testing for amdgpu offload was done via
cmake -B ../llvm_amdgpu -S llvm -GNinja -C offload/cmake/caches/Offload.cmake -DCMAKE_BUILD_TYPE=Release

PR #132252 Created a second file that shared .cpp in clang/lib/CodeGen/CMakeLists.txt

For example There were two AMDGPU.cpp's one in TargetBuiltins and the other in Targets. Even though these were in different directories libtool warns that it might not distinguish them because they share the same base name.

There are two potential fixes. The easy fix is to rename one of them and keep one cmake file. That solution though doesn't future proof this problem in the event of a third .cpp and it seems teams want to just use the target name
#132252 (comment).

The alternative fix is to seperate the cmake files into their own sub directories as static libs.

llvmbot · 2025-04-01T04:14:20Z

@llvm/pr-subscribers-hlsl
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Farzon Lotfi (farzonl)

Changes

fixes #133199

The fix to reland was moving Targets/DirectX.cpp from clang/lib/CodeGen/Targets/CMakeLists.txt back to clang/lib/CodeGen/CMakeLists.txt.

That change means that the files in clang/lib/CodeGen/Targets/CMakeLists.txt needs to be marked PARTIAL_SOURCES_INTENDED.

PR #132252 Created a second file that shared <TargetName>.cpp in clang/lib/CodeGen/CMakeLists.txt

For example There were two AMDGPU.cpp's one in TargetBuiltins and the other in Targets. Even though these were in different directories libtool warns that it might not distinguish them because they share the same base name.

There are two potential fixes. The easy fix is to rename one of them and keep one cmake file. That solution though doesn't future proof this problem in the event of a third <TargetName>.cpp and it seems teams want to just use the target name
#132252 (comment).

The alternative fix is to seperate the cmake files into their own sub directories as static libs.

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

4 Files Affected:

(modified) clang/lib/CodeGen/CMakeLists.txt (+13-36)
(modified) clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp (+1-1)
(added) clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt (+14)
(added) clang/lib/CodeGen/Targets/CMakeLists.txt (+30)

diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index ebe2fbd7db295..2bf32a6b9d8aa 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -116,45 +116,10 @@ add_clang_library(clangCodeGen
   PatternInit.cpp
   SanitizerMetadata.cpp
   SwiftCallingConv.cpp
-  TargetBuiltins/ARM.cpp
-  TargetBuiltins/AMDGPU.cpp
-  TargetBuiltins/Hexagon.cpp
-  TargetBuiltins/NVPTX.cpp
-  TargetBuiltins/PPC.cpp
-  TargetBuiltins/RISCV.cpp
-  TargetBuiltins/SPIR.cpp
-  TargetBuiltins/SystemZ.cpp
-  TargetBuiltins/WebAssembly.cpp
-  TargetBuiltins/X86.cpp
   TargetInfo.cpp
-  Targets/AArch64.cpp
-  Targets/AMDGPU.cpp
-  Targets/ARC.cpp
-  Targets/ARM.cpp
-  Targets/AVR.cpp
-  Targets/BPF.cpp
-  Targets/CSKY.cpp
   Targets/DirectX.cpp
-  Targets/Hexagon.cpp
-  Targets/Lanai.cpp
-  Targets/LoongArch.cpp
-  Targets/M68k.cpp
-  Targets/MSP430.cpp
-  Targets/Mips.cpp
-  Targets/NVPTX.cpp
-  Targets/PNaCl.cpp
-  Targets/PPC.cpp
-  Targets/RISCV.cpp
-  Targets/SPIR.cpp
-  Targets/Sparc.cpp
-  Targets/SystemZ.cpp
-  Targets/TCE.cpp
-  Targets/VE.cpp
-  Targets/WebAssembly.cpp
-  Targets/X86.cpp
-  Targets/XCore.cpp
   VarBypassDetector.cpp
-
+  
   DEPENDS
   vt_gen
   intrinsics_gen
@@ -170,4 +135,16 @@ add_clang_library(clangCodeGen
   clangFrontend
   clangLex
   clangSerialization
+  clangCodeGenTargetBuiltins
+  clangCodeGenTargets
+  )
+
+  target_include_directories(clangCodeGen
+    PUBLIC
+    ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_CURRENT_SOURCE_DIR}/TargetBuiltins
+    ${CMAKE_CURRENT_SOURCE_DIR}/Targets
   )
+  
+  add_subdirectory(TargetBuiltins)
+  add_subdirectory(Targets)
diff --git a/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp b/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
index b56b739094ff3..577fee05d4af6 100644
--- a/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
@@ -1,4 +1,4 @@
-//===------- AMDCPU.cpp - Emit LLVM Code for builtins ---------------------===//
+//===------- AMDGPU.cpp - Emit LLVM Code for builtins ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
diff --git a/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
new file mode 100644
index 0000000000000..76be68a11d02a
--- /dev/null
+++ b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
@@ -0,0 +1,14 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+add_clang_library(clangCodeGenTargetBuiltins STATIC
+  ARM.cpp
+  AMDGPU.cpp
+  Hexagon.cpp
+  NVPTX.cpp
+  PPC.cpp
+  RISCV.cpp
+  SPIR.cpp
+  SystemZ.cpp
+  WebAssembly.cpp
+  X86.cpp
+)
diff --git a/clang/lib/CodeGen/Targets/CMakeLists.txt b/clang/lib/CodeGen/Targets/CMakeLists.txt
new file mode 100644
index 0000000000000..531a9461b7201
--- /dev/null
+++ b/clang/lib/CodeGen/Targets/CMakeLists.txt
@@ -0,0 +1,30 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+add_clang_library(clangCodeGenTargets STATIC
+  PARTIAL_SOURCES_INTENDED
+  AArch64.cpp
+  AMDGPU.cpp
+  ARC.cpp
+  ARM.cpp
+  AVR.cpp
+  BPF.cpp
+  CSKY.cpp
+  Hexagon.cpp
+  Lanai.cpp
+  LoongArch.cpp
+  M68k.cpp
+  MSP430.cpp
+  Mips.cpp
+  NVPTX.cpp
+  PNaCl.cpp
+  PPC.cpp
+  RISCV.cpp
+  SPIR.cpp
+  Sparc.cpp
+  SystemZ.cpp
+  TCE.cpp
+  VE.cpp
+  WebAssembly.cpp
+  X86.cpp
+  XCore.cpp
+)

llvmbot · 2025-04-01T04:14:21Z

@llvm/pr-subscribers-backend-amdgpu

Author: Farzon Lotfi (farzonl)

Changes

fixes #133199

The fix to reland was moving Targets/DirectX.cpp from clang/lib/CodeGen/Targets/CMakeLists.txt back to clang/lib/CodeGen/CMakeLists.txt.

That change means that the files in clang/lib/CodeGen/Targets/CMakeLists.txt needs to be marked PARTIAL_SOURCES_INTENDED.

PR #132252 Created a second file that shared <TargetName>.cpp in clang/lib/CodeGen/CMakeLists.txt

For example There were two AMDGPU.cpp's one in TargetBuiltins and the other in Targets. Even though these were in different directories libtool warns that it might not distinguish them because they share the same base name.

There are two potential fixes. The easy fix is to rename one of them and keep one cmake file. That solution though doesn't future proof this problem in the event of a third <TargetName>.cpp and it seems teams want to just use the target name
#132252 (comment).

The alternative fix is to seperate the cmake files into their own sub directories as static libs.

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

4 Files Affected:

(modified) clang/lib/CodeGen/CMakeLists.txt (+13-36)
(modified) clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp (+1-1)
(added) clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt (+14)
(added) clang/lib/CodeGen/Targets/CMakeLists.txt (+30)

diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index ebe2fbd7db295..2bf32a6b9d8aa 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -116,45 +116,10 @@ add_clang_library(clangCodeGen
   PatternInit.cpp
   SanitizerMetadata.cpp
   SwiftCallingConv.cpp
-  TargetBuiltins/ARM.cpp
-  TargetBuiltins/AMDGPU.cpp
-  TargetBuiltins/Hexagon.cpp
-  TargetBuiltins/NVPTX.cpp
-  TargetBuiltins/PPC.cpp
-  TargetBuiltins/RISCV.cpp
-  TargetBuiltins/SPIR.cpp
-  TargetBuiltins/SystemZ.cpp
-  TargetBuiltins/WebAssembly.cpp
-  TargetBuiltins/X86.cpp
   TargetInfo.cpp
-  Targets/AArch64.cpp
-  Targets/AMDGPU.cpp
-  Targets/ARC.cpp
-  Targets/ARM.cpp
-  Targets/AVR.cpp
-  Targets/BPF.cpp
-  Targets/CSKY.cpp
   Targets/DirectX.cpp
-  Targets/Hexagon.cpp
-  Targets/Lanai.cpp
-  Targets/LoongArch.cpp
-  Targets/M68k.cpp
-  Targets/MSP430.cpp
-  Targets/Mips.cpp
-  Targets/NVPTX.cpp
-  Targets/PNaCl.cpp
-  Targets/PPC.cpp
-  Targets/RISCV.cpp
-  Targets/SPIR.cpp
-  Targets/Sparc.cpp
-  Targets/SystemZ.cpp
-  Targets/TCE.cpp
-  Targets/VE.cpp
-  Targets/WebAssembly.cpp
-  Targets/X86.cpp
-  Targets/XCore.cpp
   VarBypassDetector.cpp
-
+  
   DEPENDS
   vt_gen
   intrinsics_gen
@@ -170,4 +135,16 @@ add_clang_library(clangCodeGen
   clangFrontend
   clangLex
   clangSerialization
+  clangCodeGenTargetBuiltins
+  clangCodeGenTargets
+  )
+
+  target_include_directories(clangCodeGen
+    PUBLIC
+    ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_CURRENT_SOURCE_DIR}/TargetBuiltins
+    ${CMAKE_CURRENT_SOURCE_DIR}/Targets
   )
+  
+  add_subdirectory(TargetBuiltins)
+  add_subdirectory(Targets)
diff --git a/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp b/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
index b56b739094ff3..577fee05d4af6 100644
--- a/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/AMDGPU.cpp
@@ -1,4 +1,4 @@
-//===------- AMDCPU.cpp - Emit LLVM Code for builtins ---------------------===//
+//===------- AMDGPU.cpp - Emit LLVM Code for builtins ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
diff --git a/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
new file mode 100644
index 0000000000000..76be68a11d02a
--- /dev/null
+++ b/clang/lib/CodeGen/TargetBuiltins/CMakeLists.txt
@@ -0,0 +1,14 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+add_clang_library(clangCodeGenTargetBuiltins STATIC
+  ARM.cpp
+  AMDGPU.cpp
+  Hexagon.cpp
+  NVPTX.cpp
+  PPC.cpp
+  RISCV.cpp
+  SPIR.cpp
+  SystemZ.cpp
+  WebAssembly.cpp
+  X86.cpp
+)
diff --git a/clang/lib/CodeGen/Targets/CMakeLists.txt b/clang/lib/CodeGen/Targets/CMakeLists.txt
new file mode 100644
index 0000000000000..531a9461b7201
--- /dev/null
+++ b/clang/lib/CodeGen/Targets/CMakeLists.txt
@@ -0,0 +1,30 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+add_clang_library(clangCodeGenTargets STATIC
+  PARTIAL_SOURCES_INTENDED
+  AArch64.cpp
+  AMDGPU.cpp
+  ARC.cpp
+  ARM.cpp
+  AVR.cpp
+  BPF.cpp
+  CSKY.cpp
+  Hexagon.cpp
+  Lanai.cpp
+  LoongArch.cpp
+  M68k.cpp
+  MSP430.cpp
+  Mips.cpp
+  NVPTX.cpp
+  PNaCl.cpp
+  PPC.cpp
+  RISCV.cpp
+  SPIR.cpp
+  Sparc.cpp
+  SystemZ.cpp
+  TCE.cpp
+  VE.cpp
+  WebAssembly.cpp
+  X86.cpp
+  XCore.cpp
+)

farzonl · 2025-04-01T19:23:53Z

@vzakhari @asudarsa could I get another review.

nikic · 2025-04-02T15:53:39Z

Could you please provide some more detail on why the special DirectX handling is necessary? The commit message says something about "DirectX use of CodeGenModule as input to getHLSLType" being the reason, but I don't get why that is relevant.

farzonl · 2025-04-02T20:43:54Z

Could you please provide some more detail on why the special DirectX handling is necessary? The commit message says something about "DirectX use of CodeGenModule as input to getHLSLType" being the reason, but I don't get why that is relevant.

@nikic

the DirectX target is why I originally added this

target_link_libraries(clangCodeGenTargets
  PRIVATE
  clangCodeGen
)

If I didn't add this I would get two linker errors
associated with

/usr/bin/ld: lib/libclangCodeGenTargets.a(DirectX.cpp.o): in function `(anonymous namespace)::DirectXTargetCodeGenInfo::getHLSLType(clang::CodeGen::CodeGenModule&, clang::Type const*, llvm::SmallVector<int, 12u> const*) const':
DirectX.cpp:(.text._ZNK12_GLOBAL__N_124DirectXTargetCodeGenInfo11getHLSLTypeERN5clang7CodeGen13CodeGenModuleEPKNS1_4TypeEPKN4llvm11SmallVectorIiLj12EEE+0x185): undefined reference to `clang::CodeGen::HLSLBufferLayoutBuilder::createLayoutType(clang::RecordType const*, llvm::SmallVector<int, 12u> const*)'
collect2: error: ld returned 1 exit status
[135/189] Linking CXX executable bin/clang-21

and

(.text._ZNK12_GLOBAL__N_124DirectXTargetCodeGenInfo11getHLSLTypeERN5clang7CodeGen13CodeGenModuleEPKNS1_4TypeEPKN4llvm11SmallVectorIiLj12EEE+0x185): undefined reference to `clang::CodeGen::HLSLBufferLayoutBuilder::createLayoutType(clang::RecordType const*, llvm::SmallVector<int, 12u> const*)'
collect2: error: ld returned 1 exit status
[136/189] Linking CXX executable bin/clang-repl

The problem is this adds a circular dependency so then all the builds with -DBUILD_SHARED_LIBS=ON would break.

The change I have now removes the circular dependency but then forces clangCodeGen to be PARTIAL_SOURCES_INTENDED.

I could maybe avoid this but then some stuff in DirectX.cpp need to be reorganized and I was trying to avoid a code change.

nikic · 2025-04-02T21:04:58Z

I don't really get why that problem is specific to DirectX though. All of these make use of CodeGen, right?

farzonl · 2025-04-02T21:47:55Z

I don't really get why that problem is specific to DirectX though. All of these make use of CodeGen, right?

@nikic I see what you are asking. you want more specific on the error not that the error happened.

because of this:

llvm-project/clang/lib/CodeGen/Targets/DirectX.cpp

Lines 79 to 80 in 42b3f91

    
           HLSLBufferLayoutBuilder(CGM, "dx.Layout") 
        
               .createLayoutType(ContainedTy->getAsStructureType(), Packoffsets);

clang::CodeGen::HLSLBufferLayoutBuilder::createLayoutType(clang::RecordType const*, llvm::SmallVector<int, 12u> const*) which is used by DirectXTargetCodeGenInfo::getHLSLType is defined here clang/lib/CodeGen/HLSLBufferLayoutBuilder.cpp.

I could potentially try to move HLSLBufferLayoutBuilder to Targets directory and see if that fixes things.

If that works I'll ask @bogner or @hekota if its ok to move.

fixes llvm#133199 PR llvm#132252 Created a second file that shared <TargetName>.cpp in `clang/lib/CodeGen/CMakeLists.txt` For example There were two AMDGPU.cpp's one in TargetBuiltins and the other in Targets. Even though these were in different directories libtool warns that it might not distinguish them because they share the same base name. There are two fixes. The easy fix is to rename one of them and keep one cmake file. That solution though doesn't future proof this problem in the event of a third <TargetName>.cpp and it seems teams want to just use the target name llvm#132252 (comment). The alternative fix is to seperate the cmake files into their own sub directories. I chose to create static libraries. It might of been possible to build an OBJECT, but I only saw examples of $<TARGET_OBJECTS:> in compiler-rt and test directories so assumed there was a reason it wasn't used.

… so it has to be compiled in clang/lib/CodeGen/CMakeLists.txt. This means clangCodeGenTargets needs to be PARTIAL_SOURCES_INTENDED. or clang/lib/CodeGen/Targets/DirectX.cpp needs to be moved to a different directory.

…b/CodeGen/Targets fixes things so we don't need PARTIAL_SOURCES_INTENDED and don't need to special case into the clangcodegen library

hekota

LGTM!

farzonl · 2025-04-04T17:42:56Z

Awesome Thanks Helena!

@nikic are you happy with the state of this change for relanding this PR?

llvmbot added clang Clang issues not falling into any other category backend:AMDGPU clang:codegen IR generation bugs: mangling, exceptions, etc. labels Apr 1, 2025

farzonl requested a review from jhuber6 April 1, 2025 04:46

farzonl requested a review from nikic April 2, 2025 15:46

farzonl self-assigned this Apr 2, 2025

farzonl force-pushed the fix/issue-133199 branch from d00d246 to e6ec9a3 Compare April 2, 2025 21:59

llvmbot added the HLSL HLSL Language Support label Apr 2, 2025

farzonl requested a review from bogner April 2, 2025 21:59

farzonl force-pushed the fix/issue-133199 branch 2 times, most recently from 672a5f6 to baf93dd Compare April 3, 2025 11:56

farzonl added 3 commits April 4, 2025 12:32

moving HLSLBufferLayoutBuilder.cpp from clang/lib/CodeGen to clang/li…

b45f903

…b/CodeGen/Targets fixes things so we don't need PARTIAL_SOURCES_INTENDED and don't need to special case into the clangcodegen library

farzonl force-pushed the fix/issue-133199 branch from baf93dd to b45f903 Compare April 4, 2025 16:32

hekota approved these changes Apr 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland [Clang][Cmake] fix libtool duplicate member name warnings #133850

Reland [Clang][Cmake] fix libtool duplicate member name warnings #133850

farzonl commented Apr 1, 2025 •

edited

Loading

llvmbot commented Apr 1, 2025 •

edited

Loading

llvmbot commented Apr 1, 2025

farzonl commented Apr 1, 2025

nikic commented Apr 2, 2025

farzonl commented Apr 2, 2025 •

edited

Loading

nikic commented Apr 2, 2025

farzonl commented Apr 2, 2025 •

edited

Loading

hekota left a comment

farzonl commented Apr 4, 2025

Reland [Clang][Cmake] fix libtool duplicate member name warnings #133850

Are you sure you want to change the base?

Reland [Clang][Cmake] fix libtool duplicate member name warnings #133850

Conversation

farzonl commented Apr 1, 2025 • edited Loading

llvmbot commented Apr 1, 2025 • edited Loading

llvmbot commented Apr 1, 2025

farzonl commented Apr 1, 2025

nikic commented Apr 2, 2025

farzonl commented Apr 2, 2025 • edited Loading

nikic commented Apr 2, 2025

farzonl commented Apr 2, 2025 • edited Loading

hekota left a comment

Choose a reason for hiding this comment

farzonl commented Apr 4, 2025

farzonl commented Apr 1, 2025 •

edited

Loading

llvmbot commented Apr 1, 2025 •

edited

Loading

farzonl commented Apr 2, 2025 •

edited

Loading

farzonl commented Apr 2, 2025 •

edited

Loading