[PATCH] D148723: [clang] Enforce internal linkage for inline builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: jyu2, efriedma.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Otherwise we end up with link once odr because of the inline keyword, which 
translates to comdat in windows world. But we prune the body and that's 
inconsistent...
See https://godbolt.org/z/z9G87Wr37 for the original failing test case


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148723

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/inline-builtin-comdat.c


Index: clang/test/CodeGen/inline-builtin-comdat.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/inline-builtin-comdat.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-windows -S -emit-llvm %s -o - | FileCheck %s
+// Make sure we don't generate definition for Inline Builtins on windows
+
+double __cdecl frexp( double _X, int* _Y);
+inline __attribute__((always_inline))  long double __cdecl frexpl( long double 
__x, int *__exp ) {
+  return (long double) frexp((double)__x, __exp );
+}
+
+long double pain(void)
+{
+    long double f = 123.45;
+    int i;
+    long double f2 = frexpl(f, &i);
+    return f2;
+}
+
+// CHECK-NOT: define{{.*}}@frexpl(
+// CHECK: define dso_local double @pain
+// CHECK:    [[CALL_I:%.*]] = call double @frexp(double noundef [[TMP2]], ptr 
noundef [[TMP1]]) #[[ATTR3:[0-9]+]]
+// CHECK: declare dso_local double @frexp(double noundef, ptr noundef)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5189,9 +5189,10 @@
   if (D->hasAttr<WeakAttr>())
     return llvm::GlobalVariable::WeakAnyLinkage;
 
-  if (const auto *FD = D->getAsFunction())
+  if (const auto *FD = D->getAsFunction()) {
     if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
       return llvm::GlobalVariable::LinkOnceAnyLinkage;
+  }
 
   // We are guaranteed to have a strong definition somewhere else,
   // so we can use available_externally linkage.
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -11535,7 +11535,7 @@
 
 static GVALinkage basicGVALinkageForFunction(const ASTContext &Context,
                                              const FunctionDecl *FD) {
-  if (!FD->isExternallyVisible())
+  if (!FD->isExternallyVisible() || FD->isInlineBuiltinDeclaration())
     return GVA_Internal;
 
   // Non-user-provided functions get emitted as weak definitions with every



Index: clang/test/CodeGen/inline-builtin-comdat.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/inline-builtin-comdat.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-windows -S -emit-llvm %s -o - | FileCheck %s
+// Make sure we don't generate definition for Inline Builtins on windows
+
+double __cdecl frexp( double _X, int* _Y);
+inline __attribute__((always_inline))  long double __cdecl frexpl( long double __x, int *__exp ) {
+  return (long double) frexp((double)__x, __exp );
+}
+
+long double pain(void)
+{
+    long double f = 123.45;
+    int i;
+    long double f2 = frexpl(f, &i);
+    return f2;
+}
+
+// CHECK-NOT: define{{.*}}@frexpl(
+// CHECK: define dso_local double @pain
+// CHECK:    [[CALL_I:%.*]] = call double @frexp(double noundef [[TMP2]], ptr noundef [[TMP1]]) #[[ATTR3:[0-9]+]]
+// CHECK: declare dso_local double @frexp(double noundef, ptr noundef)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5189,9 +5189,10 @@
   if (D->hasAttr<WeakAttr>())
     return llvm::GlobalVariable::WeakAnyLinkage;
 
-  if (const auto *FD = D->getAsFunction())
+  if (const auto *FD = D->getAsFunction()) {
     if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)
       return llvm::GlobalVariable::LinkOnceAnyLinkage;
+  }
 
   // We are guaranteed to have a strong definition somewhere else,
   // so we can use available_externally linkage.
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -11535,7 +11535,7 @@
 
 static GVALinkage basicGVALinkageForFunction(const ASTContext &Context,
                                              const FunctionDecl *FD) {
-  if (!FD->isExternallyVisible())
+  if (!FD->isExternallyVisible() || FD->isInlineBuiltinDeclaration())
     return GVA_Internal;
 
   // Non-user-provided functions get emitted as weak definitions with every

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits