From d27b1b1afd380cb37731aa762487cc18768b2ece Mon Sep 17 00:00:00 2001 From: Andy Kaylor Date: Tue, 6 May 2025 13:31:31 -0700 Subject: [PATCH 1/2] [CIR] Add code to detect non-zero-initializable records There are some cases where we're currently using the zero attribute to initialize global variables for records that aren't properly zero-initializable. We weren't checking for that, and we also weren't properly calculating zero-initializability for records. This patch addresses both of these issues. This doesn't actually handle the case where the record isn't zero-initializable. It just reports it as NYI. It does add a comment about what needs to be done. --- clang/lib/CIR/CodeGen/CIRGenExprConst.cpp | 16 ++++++++++++-- .../CIR/CodeGen/CIRRecordLayoutBuilder.cpp | 19 ++++++++++++++++ clang/test/CIR/CodeGen/nonzeroinit-struct.cpp | 22 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 clang/test/CIR/CodeGen/nonzeroinit-struct.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp index f2bfb5cc0bf6..d9dfb37b7921 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp @@ -1717,7 +1717,7 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) { // initialization of memory to all NULLs. if (!D.hasLocalStorage()) { QualType Ty = CGM.getASTContext().getBaseElementType(D.getType()); - if (Ty->isRecordType()) + if (Ty->isRecordType()) { if (const CXXConstructExpr *E = dyn_cast_or_null(D.getInit())) { const CXXConstructorDecl *CD = E->getConstructor(); @@ -1725,9 +1725,21 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) { // just emitting a global with zero init (mimic what we do for trivial // assignments and whatnots). Since this is for globals shouldn't // be a problem for the near future. - if (CD->isTrivial() && CD->isDefaultConstructor()) + if (CD->isTrivial() && CD->isDefaultConstructor()) { + const auto *cxxrd = + cast(Ty->getAs()->getDecl()); + // Some cases, such as member pointer members, can't be zero + // initialized. The classic codegen goes through emitNullConstant + // for those cases but generates a non-zero constant. We can't + // quite do that here because we need an attribute and not a value, + // but something like that can be implemented. + if (!CGM.getTypes().isZeroInitializable(cxxrd)) { + llvm_unreachable("NYI"); + } return cir::ZeroAttr::get(CGM.convertType(D.getType())); + } } + } } InConstantContext = D.hasConstantInitialization(); diff --git a/clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp index dd325e9af5d6..ec13f93a9bf1 100644 --- a/clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp +++ b/clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp @@ -300,6 +300,7 @@ void CIRRecordLowering::lower(bool nonVirtualBaseType) { insertPadding(); members.pop_back(); + calculateZeroInit(); fillOutputFields(); computeVolatileBitfields(); } @@ -628,6 +629,24 @@ void CIRRecordLowering::accumulateFields() { } } +void CIRRecordLowering::calculateZeroInit() { + for (const MemberInfo &member : members) { + if (member.kind == MemberInfo::InfoKind::Field) { + if (!member.fieldDecl || isZeroInitializable(member.fieldDecl)) + continue; + IsZeroInitializable = IsZeroInitializableAsBase = false; + return; + } else if (member.kind == MemberInfo::InfoKind::Base || + member.kind == MemberInfo::InfoKind::VBase) { + if (isZeroInitializable(member.cxxRecordDecl)) + continue; + IsZeroInitializable = false; + if (member.kind == MemberInfo::InfoKind::Base) + IsZeroInitializableAsBase = false; + } + } +} + void CIRRecordLowering::determinePacked(bool NVBaseType) { if (isPacked) return; diff --git a/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp b/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp new file mode 100644 index 000000000000..4eebd2b7e0e4 --- /dev/null +++ b/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --input-file=%t.cir %s + +// This case is not yet implemented. +// XFAIL: * + +struct Other { + int x; +}; + +struct Trivial { + int x; + double y; + decltype(&Other::x) ptr; +}; + +// This case has a trivial default constructor, but can't be zero-initialized. +Trivial t; + +// The type is wrong on the last member here. It needs to be initialized to -1, +// but I don't know what that will look like. +// CHECK: cir.global {{.*}} @t = #cir.const_record<{#cir.int<0> : !s32i, #cir.fp<0.000000e+00> : !cir.double, #cir.int<-1> : !s64i}> From 3390087ed99f70ca943b302d85f416fec1673a94 Mon Sep 17 00:00:00 2001 From: Andy Kaylor Date: Mon, 12 May 2025 11:08:14 -0700 Subject: [PATCH 2/2] Update comment about zero-initialization --- clang/lib/CIR/CodeGen/CIRGenExprConst.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp index d9dfb37b7921..e273dd1d8297 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConst.cpp @@ -1729,10 +1729,14 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) { const auto *cxxrd = cast(Ty->getAs()->getDecl()); // Some cases, such as member pointer members, can't be zero - // initialized. The classic codegen goes through emitNullConstant - // for those cases but generates a non-zero constant. We can't - // quite do that here because we need an attribute and not a value, - // but something like that can be implemented. + // initialized. These are "zero-initialized" in the language standard + // sense, but the target ABI may require that a literal value other + // than zero be used in the initializer to make clear that a pointer + // with the value zero is not what is intended. The classic codegen + // goes through emitNullConstant for those cases but generates a + // non-zero constant. We can't quite do that here because we need an + // attribute and not a value, but something like that can be + // implemented. if (!CGM.getTypes().isZeroInitializable(cxxrd)) { llvm_unreachable("NYI"); }