[clang] [CIR] Fix calling defined functions (PR #137271)

[Andy Kaylor via cfe-commits]

https://github.com/andykaylor created https://github.com/llvm/llvm-project/pull/137271

Until now our function symbol lookup has been assuming that the function did not exist and creating a definition for it. This caused us to create a duplicate definition if we ever tried to call a function that was already defined.

This change fixes that by adding handling for trying to look up existing global definitions before creating a new one.

>From 8139104f21fc4b3b480d3cab082a212a7f83aa87 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akaylor at nvidia.com>
Date: Thu, 24 Apr 2025 16:52:55 -0700
Subject: [PATCH] [CIR] Fix calling defined functions

Until now our function symbol lookup has been assuming that the function
did not exist and creating a definition for it. This caused us to create
a duplicate definition if we ever tried to call a function that was already
defined.

This change fixes that by adding handling for trying to look up existing
global definitions before creating a new one.
---
 clang/include/clang/CIR/MissingFeatures.h |  1 +
 clang/lib/CIR/CodeGen/CIRGenModule.cpp    | 52 +++++++++++++++++++++++
 clang/test/CIR/CodeGen/call.cpp           | 11 ++---
 clang/test/CIR/CodeGen/namespace.cpp      |  5 +--
 4 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 045b9ce40f53a..bb5dac4faa1e0 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -160,6 +160,7 @@ struct MissingFeatures {
   static bool lambdaFieldToName() { return false; }
   static bool targetSpecificCXXABI() { return false; }
   static bool moduleNameHash() { return false; }
+  static bool setDSOLocal() { return false; }
 
   // Missing types
   static bool dataMemberType() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 0f4193b5756fd..e4c08328c59fd 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -773,6 +773,58 @@ cir::FuncOp CIRGenModule::getOrCreateCIRFunction(
     StringRef mangledName, mlir::Type funcType, GlobalDecl gd, bool forVTable,
     bool dontDefer, bool isThunk, ForDefinition_t isForDefinition,
     mlir::ArrayAttr extraAttrs) {
+  const Decl *d = gd.getDecl();
+
+  if (isThunk)
+    errorNYI(d->getSourceRange(), "getOrCreateCIRFunction: thunk");
+
+  // In what follows, we continue past 'errorNYI' as if nothing happened because
+  // the rest of the implementation is better than doing nothing.
+
+  // Any attempts to use a MultiVersion function should result in retrieving the
+  // iFunc instead. Name mangling will handle the rest of the changes.
+  if (const auto *fd = cast_or_null<FunctionDecl>(d)) {
+    // For the device mark the function as one that should be emitted.
+    if (getLangOpts().OpenMPIsTargetDevice && fd->isDefined() && !dontDefer &&
+        !isForDefinition)
+      errorNYI(fd->getSourceRange(),
+               "getOrCreateCIRFunction: OpenMP target function");
+
+    if (fd->isMultiVersion())
+      errorNYI(fd->getSourceRange(), "multi-version functions NYI");
+  }
+
+  // Lookup the entry, lazily creating it if necessary.
+  mlir::Operation *entry = getGlobalValue(mangledName);
+  if (entry) {
+    if (!isa<cir::FuncOp>(entry))
+      errorNYI(d->getSourceRange(), "getOrCreateCIRFunction: non-FuncOp");
+
+    assert(!cir::MissingFeatures::weakRefReference());
+
+    // Handle dropped DLL attributes.
+    if (d && !d->hasAttr<DLLImportAttr>() && !d->hasAttr<DLLExportAttr>()) {
+      assert(!cir::MissingFeatures::setDLLStorageClass());
+      assert(!cir::MissingFeatures::setDSOLocal());
+    }
+
+    // If there are two attempts to define the same mangled name, issue an
+    // error.
+    auto fn = cast<cir::FuncOp>(entry);
+    assert((!isForDefinition || !fn || !fn.isDeclaration()) &&
+           "Duplicate function definition");
+    if (fn && fn.getFunctionType() == funcType) {
+      return fn;
+    }
+
+    if (!isForDefinition) {
+      return fn;
+    }
+
+    // TODO(cir): classic codegen checks here if this is a llvm::GlobalAlias.
+    // How will we support this?
+  }
+
   auto *funcDecl = llvm::cast_or_null<FunctionDecl>(gd.getDecl());
   bool invalidLoc = !funcDecl ||
                     funcDecl->getSourceRange().getBegin().isInvalid() ||
diff --git a/clang/test/CIR/CodeGen/call.cpp b/clang/test/CIR/CodeGen/call.cpp
index 84e773ec34586..b79abb0d46efb 100644
--- a/clang/test/CIR/CodeGen/call.cpp
+++ b/clang/test/CIR/CodeGen/call.cpp
@@ -1,19 +1,20 @@
 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - 2>&1 | FileCheck %s
 
-void f1();
+void f1() {}
 void f2() {
   f1();
 }
 
-// CHECK-LABEL: cir.func @_Z2f1v
+// CHECK-LABEL:  cir.func @_Z2f1v
+// CHECK-LABEL: cir.func @_Z2f2v
 // CHECK:         cir.call @_Z2f1v() : () -> ()
 
-int f3();
+int f3() { return 2; }
 int f4() {
   int x = f3();
   return x;
 }
 
+// CHECK-LABEL: cir.func @_Z2f3v() -> !s32i
 // CHECK-LABEL: cir.func @_Z2f4v() -> !s32i
-// CHECK:         %[[#x:]] = cir.call @_Z2f3v() : () -> !s32i
-// CHECK-NEXT:    cir.store %[[#x]], %{{.+}} : !s32i, !cir.ptr<!s32i>
+// CHECK:         cir.call @_Z2f3v() : () -> !s32i
diff --git a/clang/test/CIR/CodeGen/namespace.cpp b/clang/test/CIR/CodeGen/namespace.cpp
index cfeb17bd19ced..c89adcbfc7811 100644
--- a/clang/test/CIR/CodeGen/namespace.cpp
+++ b/clang/test/CIR/CodeGen/namespace.cpp
@@ -4,10 +4,7 @@
 namespace {
   int g1 = 1;
 
-  // Note: This causes a warning about the function being undefined, but we
-  // currently have a problem with duplicate definitions when we call functions.
-  // This should be updated when that problem is fixed.
-  void f1(void);
+  void f1(void) {}
 }