Skip to content

Conversation

kikairoya
Copy link
Contributor

@kikairoya kikairoya commented Sep 13, 2025

In 45ca613, whether to mangle names based on calling conventions according to Microsoft conventions was refactored to a bool in the TargetInfo. Cygwin targets also require this mangling, but were missed, presumably due to lack of test coverage of these targets. This commit enables the name mangling for Cygwin, and also enables test coverage of this mangling on Cygwin targets.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 13, 2025

@llvm/pr-subscribers-clang

Author: Tomohiro Kashiwada (kikairoya)

Changes

follow-up to 45ca613


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

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/X86.h (+2)
  • (modified) clang/test/CodeGen/mangle-windows.c (+4-2)
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 6e013c95dbf01..d159a7906854c 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -646,6 +646,7 @@ class LLVM_LIBRARY_VISIBILITY CygwinX86_32TargetInfo : public X86_32TargetInfo {
       : X86_32TargetInfo(Triple, Opts) {
     this->WCharType = TargetInfo::UnsignedShort;
     this->WIntType = TargetInfo::UnsignedInt;
+    this->UseMicrosoftManglingForC = true;
     DoubleAlign = LongLongAlign = 64;
     resetDataLayout("e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:64-i64:64-"
                     "i128:128-f80:32-n8:16:32-a:0:32-S32",
@@ -983,6 +984,7 @@ class LLVM_LIBRARY_VISIBILITY CygwinX86_64TargetInfo : public X86_64TargetInfo {
       : X86_64TargetInfo(Triple, Opts) {
     this->WCharType = TargetInfo::UnsignedShort;
     this->WIntType = TargetInfo::UnsignedInt;
+    this->UseMicrosoftManglingForC = true;
   }
 
   void getTargetDefines(const LangOptions &Opts,
diff --git a/clang/test/CodeGen/mangle-windows.c b/clang/test/CodeGen/mangle-windows.c
index 046b1e8815a8a..e1b06e72a9635 100644
--- a/clang/test/CodeGen/mangle-windows.c
+++ b/clang/test/CodeGen/mangle-windows.c
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-mingw32 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-mingw32  | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-cygwin   | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=i386-pc-windows-msvc-elf | FileCheck %s --check-prefix=ELF32
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s --check-prefix=X64
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-mingw32 | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-mingw32  | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-cygwin   | FileCheck %s --check-prefix=X64
 // RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-pc-windows-msvc-elf | FileCheck %s --check-prefix=ELF64
 
 // CHECK: target datalayout = "e-m:x-{{.*}}"

@jeremyd2019
Copy link
Contributor

Consider also updating clang/test/CodeGenCXX/mangle-windows.cpp to add i386-cygwin with ITANIUM prefix?

Copy link
Contributor

@jeremyd2019 jeremyd2019 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @mstorsjo I believe this is a regression from 20.1.x (the commit cited only appears with llvm 21 tags), does that make this eligible for cherry-picking to 21.x?

@jeremyd2019
Copy link
Contributor

The description could use some improvement. How about something like

In 45ca613, whether to mangle names based on calling conventions according to Microsoft conventions was refactored to a bool in the TargetInfo. Cygwin targets also require this mangling, but were missed, presumably due to lack of test coverage of these targets. This commit enables the name mangling for Cygwin, and also enables test coverage of this mangling on Cygwin targets.

@mstorsjo mstorsjo merged commit 4abcbb0 into llvm:main Sep 13, 2025
9 checks passed
@mstorsjo mstorsjo added this to the LLVM 21.x Release milestone Sep 13, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Sep 13, 2025
@mstorsjo
Copy link
Member

/cherry-pick 4abcbb0

@llvmbot
Copy link
Member

llvmbot commented Sep 13, 2025

/pull-request #158442

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Sep 13, 2025
@kikairoya kikairoya deleted the cygwin-microsoft-mangle branch September 13, 2025 22:40
jeremyd2019 pushed a commit to jeremyd2019/llvm-project that referenced this pull request Sep 14, 2025
In
llvm@45ca613,
whether to mangle names based on calling conventions according to
Microsoft conventions was refactored to a bool in the TargetInfo. Cygwin
targets also require this mangling, but were missed, presumably due to
lack of test coverage of these targets. This commit enables the name
mangling for Cygwin, and also enables test coverage of this mangling on
Cygwin targets.

(cherry picked from commit 4abcbb0)
tru pushed a commit that referenced this pull request Sep 15, 2025
In
45ca613,
whether to mangle names based on calling conventions according to
Microsoft conventions was refactored to a bool in the TargetInfo. Cygwin
targets also require this mangling, but were missed, presumably due to
lack of test coverage of these targets. This commit enables the name
mangling for Cygwin, and also enables test coverage of this mangling on
Cygwin targets.

(cherry picked from commit 4abcbb0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

4 participants