[RFC] Rename source files in clang/lib/CodeGen/TargetBuiltins/*

Clang Frontend
,
1

I start to hiting clang build warnings on macOS 12/15 sometime earlier this year:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (AMDGPU.cpp.o) in output file used for input files: tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/TargetBuiltins/AMDGPU.cpp.o and: tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/Targets/AMDGPU.cpp.o (due to use of basename, truncation, blank padding or duplicate input files)
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ARM.cpp.o) in output file used for input files: tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/TargetBuiltins/ARM.cpp.o and: tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/Targets/ARM.cpp.o (due to use of basename, truncation, blank padding or duplicate input files)
.....

Seems it was introduced by [NFC][clang] Split clang/lib/CodeGen/CGBuiltin.cpp into target-specific files by jthackray · Pull Request #132252 · llvm/llvm-project · GitHub.

After rename these files, the build warning cleaned.

image
image1920×587 153 KB

image
image1920×1269 188 KB

2

@jthackray

3

I’d like to see some sort of discussion here. For me, I don’t think the name change really bothers me, though I have a slight preference for TargetBuiltinXXX.cpp rather than TargetBuiltinsXXX.cpp

For the most part I don’t have a problem with it, but I fear it is papering over a more significant problem: that our output files are being named with JUST the file name, instead of directory names/etc.

That said, is there reasonable justification for that diagnostic? I find myself wondering: Whats the harm?

4

I have only seen this warning on macOS and not on Linux or Windows. IIUC, seems the root cause is macOS libtool, and libtool emit diagnostics based only on the file name but not the full path.

I have a slight preference for TargetBuiltinXXX.cpp rather than TargetBuiltinsXXX.cpp

These two all looks good to me, or a shorter name? I have no preference for these.

In addition to clang/lib/CodeGen/TargetBuiltins, should the files in clang/lib/CodeGen/Target/ also be renamed to CGX86.cpp, CGArm.cpp, etc., just like the file names in other CodeGen directories?

5

Is anyone familiar with Apple’s toolchain?

6

This appears to be specific to MacOS Xcode, and it appears to be complaining because there are 2 object files both called AMDGPU.cpp.o (ditto for other architectures) but in different directories. Searching the web, this warning has been present for over 20 years, and there don’t appear to be alternatives to renaming the files to avoid this warning.

I wasn’t aware of this issue when I merged the PR, since there isn’t a MacOS builbot builder, so I didn’t see this warning. If renaming these files, a shorter prefix is preferable to a longer name.

Could we just rename them to BuiltinARM.cpp, etc? Or TargetARM.cpp?

It looks like an issue has already been filed as [MacOS 15][AppleClang] libtool duplicate member name warnings · Issue #133199 · llvm/llvm-project · GitHub

7

Here’s a fix, comments welcome: [clang] Rename files that MacOS libtool warns about (NFC) by jthackray · Pull Request #150054 · llvm/llvm-project · GitHub

8

Presumably a fix should also be cherry-picked into the llvm21 branch too, to avoid annoying MacOS users forever?

1 Like
9

Haha, seems we create It looks like we created a duplicate PR. [NFC][clang] Rename clang/lib/CodeGen/TargetBuiltins/* files to avoid build warnings on macOS by yronglin · Pull Request #149974 · llvm/llvm-project · GitHub

10

This sounds good to me.

11

This only affects people who are BUILDING LLVM/Clang, right? I don’t really see a reason to cherry-pick it into the branch if that is the case. This is one of those ‘annoys llvm/clang devs’, which aren’t cherry-pick worthy.

1 Like
12

I think we should also rename files in clang/lib/CodeGen/Target/ to follow other files in CodeGen. But this is probably outside the scope of this RFC

1 Like
13

Yes, it’s only impact people who want to build clang on macOS.

14

I want to suggest that we could just use llvm ar instead of libtool, to avoid this problem now and ever again in the future.

But I guess requiring that you build llvm tools for the host before building llvm is probably not a popular answer. :slight_smile: