Skip to content

Commit b4d4611

Browse files
authored
[CIR][CIRGen] Fix "definition with same mangled name" error (#1016)
We had some incorrect logic when creating functions and getting their address which resulted in spurious "definition with the same mangled name" errors. Fix that logic to match original CodeGen, which also fixes these errors. It's expected that decls can appear in the deferred decl list multiple times, and CodeGen has to guard against that. In the case that triggered the error, both `CIRGenerator::HandleInlineFunctionDefinition` and CIRGenModule were deferring the declaration. Something else I discovered here is that we emit these functions in the opposite order as regular CodeGen: https://godbolt.org/z/4PrKG7h9b. That might be a meaningful difference worth investigating further. Fixes #991
1 parent 29e5998 commit b4d4611

File tree

2 files changed

+27
-14
lines changed

2 files changed

+27
-14
lines changed

clang/lib/CIR/CodeGen/CIRGenModule.cpp

+12-14
Original file line numberDiff line numberDiff line change
@@ -607,20 +607,17 @@ void CIRGenModule::buildGlobalFunctionDefinition(GlobalDecl GD,
607607
auto Ty = getTypes().GetFunctionType(FI);
608608

609609
// Get or create the prototype for the function.
610-
// if (!V || (V.getValueType() != Ty))
611-
// TODO(cir): Figure out what to do here? llvm uses a GlobalValue for the
612-
// FuncOp in mlir
613-
Op = GetAddrOfFunction(GD, Ty, /*ForVTable=*/false, /*DontDefer=*/true,
614-
ForDefinition);
615-
616-
auto globalVal = dyn_cast_or_null<mlir::cir::CIRGlobalValueInterface>(Op);
617-
if (globalVal && !globalVal.isDeclaration()) {
618-
// Already emitted.
610+
auto Fn = dyn_cast_if_present<mlir::cir::FuncOp>(Op);
611+
if (!Fn || Fn.getFunctionType() != Ty)
612+
Fn = GetAddrOfFunction(GD, Ty, /*ForVTable=*/false, /*DontDefer=*/true,
613+
ForDefinition);
614+
615+
// Already emitted.
616+
if (!Fn.isDeclaration())
619617
return;
620-
}
621-
auto Fn = cast<mlir::cir::FuncOp>(Op);
618+
622619
setFunctionLinkage(GD, Fn);
623-
setGVProperties(Op, D);
620+
setGVProperties(Fn, D);
624621
// TODO(cir): MaubeHandleStaticInExternC
625622
// TODO(cir): maybeSetTrivialComdat
626623
// TODO(cir): setLLVMFunctionFEnvAttributes
@@ -633,7 +630,7 @@ void CIRGenModule::buildGlobalFunctionDefinition(GlobalDecl GD,
633630
}
634631
CurCGF = nullptr;
635632

636-
setNonAliasAttributes(GD, Op);
633+
setNonAliasAttributes(GD, Fn);
637634
setCIRFunctionAttributesForDefinition(D, Fn);
638635

639636
if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>())
@@ -2672,7 +2669,8 @@ mlir::cir::FuncOp CIRGenModule::GetOrCreateCIRFunction(
26722669
// CHeck that GD is not yet in DiagnosedConflictingDefinitions is required
26732670
// to make sure that we issue and error only once.
26742671
if (lookupRepresentativeDecl(MangledName, OtherGD) &&
2675-
(GD.getCanonicalDecl().getDecl()) &&
2672+
(GD.getCanonicalDecl().getDecl() !=
2673+
OtherGD.getCanonicalDecl().getDecl()) &&
26762674
DiagnosedConflictingDefinitions.insert(GD).second) {
26772675
getDiags().Report(D->getLocation(), diag::err_duplicate_mangled_name)
26782676
<< MangledName;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
2+
// RUN: FileCheck --input-file=%t.cir %s
3+
4+
// This would previously emit a "definition with same mangled name as another
5+
// definition" error: https://github.com/llvm/clangir/issues/991.
6+
namespace N {
7+
struct S {
8+
// CHECK: cir.func linkonce_odr @_ZN1N1S3fooEv({{.*}} {
9+
void foo() {}
10+
};
11+
12+
// CHECK: cir.func @_ZN1N1fEv() {{.*}} {
13+
// CHECK: cir.call @_ZN1N1S3fooEv(
14+
void f() { S().foo(); }
15+
} // namespace N

0 commit comments

Comments
 (0)