https://github.com/erichkeane created https://github.com/llvm/llvm-project/pull/223444
This popped up in a benchmark. Classic codegen ALWAYS sets this as 'excludeCtor'/'excludeDtor', but CIR tried 'harder' to get this right. However, with an incomplete type, we can't actually look into it to see if it has any mutable members, so determining 'constness' isn't possible. We could PROBABLY do better with enums since I would assume they can be constant most of the time, but we fall-back to classic-codegen's behavior here instead: which marks it as non-const. Thanks to 'trying harder' than classic codegen as mentioned above, there IS a slight LLVM-IR difference, but for the better. Also note: Claude helped me with the tests. >From 950df7081e1da509ebf45c907a21775381f7ec7d Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Mon, 14 Sep 2026 08:45:15 -0700 Subject: [PATCH] [CIR] Fix 'isConstant' calc when looking at an incomplete type This popped up in a benchmark. Classic codegen ALWAYS sets this as 'excludeCtor'/'excludeDtor', but CIR tried 'harder' to get this right. However, with an incomplete type, we can't actually look into it to see if it has any mutable members, so determining 'constness' isn't possible. We could PROBABLY do better with enums since I would assume they can be constant most of the time, but we fall-back to classic-codegen's behavior here instead: which marks it as non-const. Thanks to 'trying harder' than classic codegen as mentioned above, there IS a slight LLVM-IR difference, but for the better. Also note: Claude helped me with the tests. --- clang/lib/CIR/CodeGen/CIRGenModule.cpp | 13 +++++- clang/test/CIR/CodeGen/forward-decls.cpp | 58 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 6da830289568e..bdc2707723283 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -1294,10 +1294,19 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty, // at creation time, matching the logic used in emitCXXGlobalVarDeclInit. bool isConstant = false; if (d) { + QualType declType = d->getType(); + + // Classic codegen doesn't try to exclude ctor or dtor here, but has a FIXME + // to try to do a better job. So this bit of code does slightly more effort + // to get a more accurate answer. We can try to exclude ctor, but only when + // the type is complete, as otherwise we have to check for + // fields(particularly whether they are mutable). + bool excludeCtor = !declType->isIncompleteType(); bool needsDtor = d->needsDestruction(astContext) == QualType::DK_cxx_destructor; - isConstant = d->getType().isConstantStorage( - astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/!needsDtor); + + isConstant = declType.isConstantStorage(astContext, excludeCtor, + /*ExcludeDtor=*/!needsDtor); } mlir::ptr::MemorySpaceAttrInterface declCIRAS = diff --git a/clang/test/CIR/CodeGen/forward-decls.cpp b/clang/test/CIR/CodeGen/forward-decls.cpp index ef5fe16f9566f..3394913803182 100644 --- a/clang/test/CIR/CodeGen/forward-decls.cpp +++ b/clang/test/CIR/CodeGen/forward-decls.cpp @@ -122,3 +122,61 @@ struct A { } b; }; void test(struct A *a){}; + + +//--- incomplete_class_comma_expr +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ -fclangir -emit-cir %t/incomplete_class_comma_expr -o %t/incomplete_class_comma_expr.cir +// RUN: FileCheck %s --input-file=%t/incomplete_class_comma_expr.cir --check-prefix=CHECK6 +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ -fclangir -emit-llvm %t/incomplete_class_comma_expr -o %t/incomplete_class_comma_expr-cir.ll +// RUN: FileCheck %s --input-file=%t/incomplete_class_comma_expr-cir.ll --check-prefix=CIR6 +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ -emit-llvm %t/incomplete_class_comma_expr -o %t/incomplete_class_comma_expr-ogcg.ll +// RUN: FileCheck %s --input-file=%t/incomplete_class_comma_expr-ogcg.ll --check-prefix=OGCG6 + +class Enum extern const writeTypeNames; +int flags = (writeTypeNames, flags); + +// Incomplete type, never constant even though its an enum. +// CHECK6: cir.global "private" external @writeTypeNames : !rec_Enum +// CHECK6-NOT: constant + +// CIR6: @writeTypeNames = external global %class.Enum +// CIR6-NOT: constant + +// OGCG6: @writeTypeNames = external global %class.Enum +// OGCG6-NOT: constant + +//--- extern_const_global_constant +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ -fclangir -emit-cir %t/extern_const_global_constant -o %t/extern_const_global_constant.cir +// RUN: FileCheck %s --input-file=%t/extern_const_global_constant.cir --check-prefix=CHECK7 +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ -fclangir -emit-llvm %t/extern_const_global_constant -o %t/extern_const_global_constant-cir.ll +// RUN: FileCheck %s --input-file=%t/extern_const_global_constant-cir.ll --check-prefix=CIR7 +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -x c++ -emit-llvm %t/extern_const_global_constant -o %t/extern_const_global_constant-ogcg.ll +// RUN: FileCheck %s --input-file=%t/extern_const_global_constant-ogcg.ll --check-prefix=OGCG7 + +// Complete class with no mutable fields, can be marked constant. +struct NoMutable { int x; }; +extern const NoMutable no_mutable_val; +// CHECK7: cir.global "private" constant external @no_mutable_val : !rec_NoMutable +// Note: This is a case where CIR is doing a better job than classic, which +// always fails to exclude ctor/dtor. +// CIR7: @no_mutable_val = external constant %struct.NoMutable +// OGCG7: @no_mutable_val = external global %struct.NoMutable + +// Complete class with a mutable field. +struct WithMutable { mutable int x; }; +extern const WithMutable with_mutable_val; +// CHECK7: cir.global "private" external @with_mutable_val : !rec_WithMutable +// CHECK7-NOT: constant +// CIR7: @with_mutable_val = external global %struct.WithMutable +// CIR7-NOT: constant +// OGCG7: @with_mutable_val = external global %struct.WithMutable + +// Incomplete class - cannot be marked constant. +class Incomplete; +extern const Incomplete incomplete_val; +// CHECK7: cir.global "private" external @incomplete_val : !rec_Incomplete +// CIR7: @incomplete_val = external global %class.Incomplete +// OGCG7: @incomplete_val = external global %class.Incomplete + +void use(const NoMutable &, const WithMutable &, const Incomplete *); +void foo() { use(no_mutable_val, with_mutable_val, &incomplete_val); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
