llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/137271.diff 4 Files Affected: - (modified) clang/include/clang/CIR/MissingFeatures.h (+1) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+52) - (modified) clang/test/CIR/CodeGen/call.cpp (+6-5) - (modified) clang/test/CIR/CodeGen/namespace.cpp (+1-4) ``````````diff 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) {} } `````````` </details> https://github.com/llvm/llvm-project/pull/137271 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
