llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Erich Keane (erichkeane) <details> <summary>Changes</summary> My last patch, #<!-- -->228586, did some additional work to make sure we properly initialized globals even if they had multiple blocks in their initializer rather than assuming there was one. As a result, the dynamic initialization of a variable referenced multiple times regressed, as we started failing verification because we are double-emitting the initializer. Note we ALWAYS double-emitted the initializer, but we replaced it every time. The change above caused us to re-use the same set of blocks to better tolerate multiple blocks, and as a side effect, the second block initialization added instead of replacing. This patch fixes this by making sure we only define a global variable if it isn't yet defined. --- Full diff: https://github.com/llvm/llvm-project/pull/230279.diff 2 Files Affected: - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+7) - (modified) clang/test/CIR/CodeGen/global-var-template-ctor.cpp (+29-11) ``````````diff diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 1b17ad636829d5..f399bfd3b30459 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -673,6 +673,13 @@ void CIRGenModule::emitGlobal(clang::GlobalDecl gd) { // to benefit from cache locality. Deferring code generation is necessary to // avoid adding initializers to external declarations. if (mustBeEmitted(global) && mayBeEmittedEagerly(global)) { + // Make sure we don't double-emit this declaration. + if (auto existing = dyn_cast_if_present<cir::CIRGlobalValueInterface>( + getGlobalValue(getMangledName(gd))); + existing && !existing.isDeclaration()) + if (!existing.isDeclaration()) + return; + // Emit the definition if it can't be deferred. emitGlobalDefinition(gd); return; diff --git a/clang/test/CIR/CodeGen/global-var-template-ctor.cpp b/clang/test/CIR/CodeGen/global-var-template-ctor.cpp index a01d7d7431a32f..dae27e4415ce1e 100644 --- a/clang/test/CIR/CodeGen/global-var-template-ctor.cpp +++ b/clang/test/CIR/CodeGen/global-var-template-ctor.cpp @@ -1,9 +1,9 @@ // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir // RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll -// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s +// RUN: FileCheck --check-prefix=LLVM,LLVMCIR --input-file=%t-cir.ll %s // RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll -// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s +// RUN: FileCheck --check-prefix=LLVM,OGCG --input-file=%t.ll %s // Regression test: C++14 variable templates with non-constexpr // constructors must not crash CIR with "ctorRegion failed to @@ -23,14 +23,32 @@ int test() { return fix<1> + fix<2>; } -// CIR: cir.global "private" internal dso_local @_ZL3fixILi1EE -// CIR: cir.global "private" internal dso_local @_ZL3fixILi2EE -// CIR: cir.func {{.*}} @_Z4testv +// make sure we only emit the initializer 1x, even if it is used 2x. +int get_value(); +template<int N> int dyn = get_value(); -// LLVM: @_ZL3fixILi1EE = internal global -// LLVM: @_ZL3fixILi2EE = internal global -// LLVM: define {{.*}} @_Z4testv +int test_multi_use() { + return dyn<1> + dyn<1>; +} +// CIR-DAG: cir.global "private" internal dso_local @_ZL3fixILi1EE = #cir.zero : !rec_FixedInt3C13E +// CIR-DAG: cir.global "private" internal dso_local @_ZL3fixILi2EE = #cir.zero : !rec_FixedInt3C23E +// CIR-DAG: cir.global linkonce_odr comdat dynamic_init_guard<"_ZGV3dynILi1EE"> @_Z3dynILi1EE = #cir.int<0> : !s32i + +// LLVM-DAG: @_ZL3fixILi1EE = internal global %{{.*}}FixedInt{{.*}} zeroinitializer +// LLVM-DAG: @_ZL3fixILi2EE = internal global %{{.*}}FixedInt{{.*}} zeroinitializer +// LLVM-DAG: @_ZGV3dynILi1EE = linkonce_odr global i64 0, comdat($_Z3dynILi1EE) + +// Classic orderes this first for some reason, so we have to have a separate +// check line. +// OGCG-DAG: define {{.*}} @_Z4testv + +// CIR-LABEL: cir.func comdat("_Z3dynILi1EE") {{.*}}@__cxx_global_var_init +// CIR: cir.call @_Z9get_valuev() +// CIR-NOT: cir.call @_Z9get_valuev() + +// LLVM-LABEL: define internal void @__cxx_global_var_init{{.*}}() {{.*}}comdat($_Z3dynILi1EE) { +// LLVM: call {{.*}}@_Z9get_valuev() +// LLVM-NOT: call {{.*}}@_Z9get_valuev() -// OGCG: @_ZL3fixILi1EE = internal global -// OGCG: @_ZL3fixILi2EE = internal global -// OGCG: define {{.*}} @_Z4testv +// CIR-DAG: cir.func {{.*}} @_Z4testv +// LLVMCIR-DAG: define {{.*}} @_Z4testv `````````` </details> https://github.com/llvm/llvm-project/pull/230279 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
