https://github.com/andykaylor updated
https://github.com/llvm/llvm-project/pull/138368
>From 483efe53c40d12895dc264fa993d3b286f8d85b2 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akay...@nvidia.com>
Date: Tue, 29 Apr 2025 12:59:13 -0700
Subject: [PATCH 1/2] [CIR] Unblock simple C++ structure support
This change adds additional checks to a few places where a simple struct in
C++ code was triggering `errorNYI` in places where no additional handling
was needed, and adds a very small amount of trivial initialization. The code
now checks for the conditions that do require extra handling before issuing
the diagnostic.
New tests are added for declaring and using a simple struct in C++ code.
---
clang/lib/CIR/CodeGen/CIRGenExpr.cpp | 9 +++--
clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 13 +++++--
.../CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp | 25 +++++++++----
clang/lib/CIR/CodeGen/CIRGenTypes.cpp | 7 +++-
clang/test/CIR/CodeGen/struct.cpp | 37 +++++++++++++++++++
5 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 94a6c03f7f1a4..64cbda2ebe0af 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -322,9 +322,12 @@ LValue CIRGenFunction::emitLValueForField(LValue base,
const FieldDecl *field) {
assert(!cir::MissingFeatures::opTBAA());
Address addr = base.getAddress();
- if (isa<CXXRecordDecl>(rec)) {
- cgm.errorNYI(field->getSourceRange(), "emitLValueForField: C++ class");
- return LValue();
+ if (auto *classDecl = dyn_cast<CXXRecordDecl>(rec)) {
+ if (cgm.getCodeGenOpts().StrictVTablePointers &&
+ classDecl->isDynamicClass()) {
+ cgm.errorNYI(field->getSourceRange(),
+ "emitLValueForField: strict vtable for dynamic class");
+ }
}
unsigned recordCVR = base.getVRQualifiers();
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index ab1ea07bbf5ef..9e1e2e4dd6b58 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -365,10 +365,15 @@ mlir::Attribute
ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &d) {
if (!d.hasLocalStorage()) {
QualType ty = cgm.getASTContext().getBaseElementType(d.getType());
if (ty->isRecordType())
- if (d.getInit() && isa<CXXConstructExpr>(d.getInit())) {
- cgm.errorNYI(d.getInit()->getBeginLoc(),
- "tryEmitPrivateForVarInit CXXConstructExpr");
- return {};
+ if (const CXXConstructExpr *e =
+ dyn_cast_or_null<CXXConstructExpr>(d.getInit())) {
+ const CXXConstructorDecl *cd = e->getConstructor();
+ // FIXME: we should probably model this more closely to C++ than
+ // 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())
+ return cir::ZeroAttr::get(cgm.convertType(d.getType()));
}
}
inConstantContext = d.hasConstantInitialization();
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 5bcd408b4072a..2b95d2e12014c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -177,18 +177,26 @@ void CIRRecordLowering::lower() {
return;
}
- if (isa<CXXRecordDecl>(recordDecl)) {
- cirGenTypes.getCGModule().errorNYI(recordDecl->getSourceRange(),
- "lower: class");
- return;
- }
-
assert(!cir::MissingFeatures::cxxSupport());
CharUnits size = astRecordLayout.getSize();
accumulateFields();
+ if (auto const *cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordDecl)) {
+ if (cxxRecordDecl->getNumBases() > 0) {
+ CIRGenModule &cgm = cirGenTypes.getCGModule();
+ cgm.errorNYI(recordDecl->getSourceRange(),
+ "CIRRecordLowering::lower: derived CXXRecordDecl");
+ return;
+ }
+ if (members.empty()) {
+ appendPaddingBytes(size);
+ assert(!cir::MissingFeatures::bitfields());
+ return;
+ }
+ }
+
llvm::stable_sort(members);
// TODO: implement clipTailPadding once bitfields are implemented
assert(!cir::MissingFeatures::bitfields());
@@ -295,7 +303,10 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd,
cir::RecordType *ty) {
// If we're in C++, compute the base subobject type.
if (llvm::isa<CXXRecordDecl>(rd) && !rd->isUnion() &&
!rd->hasAttr<FinalAttr>()) {
- cgm.errorNYI(rd->getSourceRange(), "computeRecordLayout: CXXRecordDecl");
+ if (lowering.astRecordLayout.getNonVirtualSize() !=
+ lowering.astRecordLayout.getSize()) {
+ cgm.errorNYI(rd->getSourceRange(), "computeRecordLayout: CXXRecordDecl");
+ }
}
// Fill in the record *after* computing the base type. Filling in the body
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index e85f2f4aa0978..ef17d622f1d27 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -237,8 +237,11 @@ mlir::Type CIRGenTypes::convertRecordDeclType(const
clang::RecordDecl *rd) {
assert(insertResult && "isSafeToCovert() should have caught this.");
// Force conversion of non-virtual base classes recursively.
- if (isa<CXXRecordDecl>(rd)) {
- cgm.errorNYI(rd->getSourceRange(), "CXXRecordDecl");
+ if (const auto *cxxRecordDecl = dyn_cast<CXXRecordDecl>(rd)) {
+ if (cxxRecordDecl->getNumBases() > 0) {
+ cgm.errorNYI(rd->getSourceRange(),
+ "convertRecordDeclType: derived CXXRecordDecl");
+ }
}
// Layout fields.
diff --git a/clang/test/CIR/CodeGen/struct.cpp
b/clang/test/CIR/CodeGen/struct.cpp
index 0d939ddd0b338..208d8f184475c 100644
--- a/clang/test/CIR/CodeGen/struct.cpp
+++ b/clang/test/CIR/CodeGen/struct.cpp
@@ -12,6 +12,17 @@ IncompleteS *p;
// LLVM: @p = dso_local global ptr null
// OGCG: @p = global ptr null, align 8
+struct CompleteS {
+ int a;
+ char b;
+};
+
+CompleteS cs;
+
+// CIR: cir.global external @cs = #cir.zero : !rec_CompleteS
+// LLVM-DAG: @cs = dso_local global %struct.CompleteS zeroinitializer
+// OGCG-DAG: @cs = global %struct.CompleteS zeroinitializer, align 4
+
void f(void) {
IncompleteS *p;
}
@@ -28,3 +39,29 @@ void f(void) {
// OGCG-NEXT: entry:
// OGCG-NEXT: %[[P:.*]] = alloca ptr, align 8
// OGCG-NEXT: ret void
+
+char f2(CompleteS &s) {
+ return s.b;
+}
+
+// CIR: cir.func @_Z2f2R9CompleteS(%[[ARG_S:.*]]:
!cir.ptr<!rec_CompleteS>{{.*}})
+// CIR: %[[S_ADDR:.*]] = cir.alloca !cir.ptr<!rec_CompleteS>,
!cir.ptr<!cir.ptr<!rec_CompleteS>>, ["s", init, const]
+// CIR: cir.store %[[ARG_S]], %[[S_ADDR]]
+// CIR: %[[S_REF:.*]] = cir.load %[[S_ADDR]]
+// CIR: %[[S_ADDR2:.*]] = cir.get_member %[[S_REF]][1] {name = "b"}
+// CIR: %[[S_B:.*]] = cir.load %[[S_ADDR2]]
+
+// LLVM: define i8 @_Z2f2R9CompleteS(ptr %[[ARG_S:.*]])
+// LLVM: %[[S_ADDR:.*]] = alloca ptr
+// LLVM: store ptr %[[ARG_S]], ptr %[[S_ADDR]]
+// LLVM: %[[S_REF:.*]] = load ptr, ptr %[[S_ADDR]], align 8
+// LLVM: %[[S_ADDR2:.*]] = getelementptr %struct.CompleteS, ptr %[[S_REF]],
i32 0, i32 1
+// LLVM: %[[S_B:.*]] = load i8, ptr %[[S_ADDR2]]
+
+// OGCG: define{{.*}} i8 @_Z2f2R9CompleteS(ptr{{.*}} %[[ARG_S:.*]])
+// OGCG: entry:
+// OGCG: %[[S_ADDR:.*]] = alloca ptr
+// OGCG: store ptr %[[ARG_S]], ptr %[[S_ADDR]]
+// OGCG: %[[S_REF:.*]] = load ptr, ptr %[[S_ADDR]]
+// OGCG: %[[S_ADDR2:.*]] = getelementptr inbounds nuw %struct.CompleteS, ptr
%[[S_REF]], i32 0, i32 1
+// OGCG: %[[S_B:.*]] = load i8, ptr %[[S_ADDR2]]
>From a160ec945c6f18ec9cebf74389bada44e48e9708 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akay...@nvidia.com>
Date: Mon, 5 May 2025 14:47:56 -0700
Subject: [PATCH 2/2] Add check for base class and test for unions
---
clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 21 +++++--
clang/lib/CIR/CodeGen/CIRGenRecordLayout.h | 26 +++++++-
.../CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp | 21 ++++++-
clang/test/CIR/CodeGen/union.cpp | 59 +++++++++++++++++++
4 files changed, 119 insertions(+), 8 deletions(-)
create mode 100644 clang/test/CIR/CodeGen/union.cpp
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index 9e1e2e4dd6b58..b6c39d96509a0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -14,6 +14,7 @@
#include "CIRGenConstantEmitter.h"
#include "CIRGenFunction.h"
#include "CIRGenModule.h"
+#include "CIRGenRecordLayout.h"
#include "mlir/IR/Attributes.h"
#include "mlir/IR/BuiltinAttributeInterfaces.h"
#include "mlir/IR/BuiltinAttributes.h"
@@ -364,17 +365,29 @@ 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 (const CXXConstructExpr *e =
- dyn_cast_or_null<CXXConstructExpr>(d.getInit())) {
+ if (ty->isRecordType()) {
+ if (const auto *e = dyn_cast_or_null<CXXConstructExpr>(d.getInit())) {
const CXXConstructorDecl *cd = e->getConstructor();
// FIXME: we should probably model this more closely to C++ than
// 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<CXXRecordDecl>(ty->getAs<RecordType>()->getDecl());
+ if (cxxrd->getNumBases() != 0) {
+ // There may not be anything additional to do here, but this will
+ // force us to pause and test this path when it is supported.
+ cgm.errorNYI("tryEmitPrivateForVarInit: cxx record with bases");
+ return {};
+ }
+ assert(cgm.getTypes()
+ .getCIRGenRecordLayout(cxxrd)
+ .isZeroInitializable());
return cir::ZeroAttr::get(cgm.convertType(d.getType()));
+ }
}
+ }
}
inConstantContext = d.hasConstantInitialization();
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
b/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
index 11768b042e87e..2ece85b8aa0a3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayout.h
@@ -33,9 +33,23 @@ class CIRGenRecordLayout {
/// field no. This info is populated by the record builder.
llvm::DenseMap<const clang::FieldDecl *, unsigned> fieldIdxMap;
+ /// False if any direct or indirect subobject of this class, when considered
+ /// as a complete object, requires a non-zero bitpattern when
+ /// zero-initialized.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned zeroInitializable : 1;
+
+ /// False if any direct or indirect subobject of this class, when considered
+ /// as a base subobject, requires a non-zero bitpattern when
zero-initialized.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned zeroInitializableAsBase : 1;
+
public:
- CIRGenRecordLayout(cir::RecordType completeObjectType)
- : completeObjectType(completeObjectType) {}
+ CIRGenRecordLayout(cir::RecordType completeObjectType, bool
zeroInitializable,
+ bool zeroInitializableAsBase)
+ : completeObjectType(completeObjectType),
+ zeroInitializable(zeroInitializable),
+ zeroInitializableAsBase(zeroInitializableAsBase) {}
/// Return the "complete object" LLVM type associated with
/// this record.
@@ -47,6 +61,14 @@ class CIRGenRecordLayout {
assert(fieldIdxMap.count(fd) && "Invalid field for record!");
return fieldIdxMap.lookup(fd);
}
+
+ /// Check whether this struct can be C++ zero-initialized
+ /// with a zeroinitializer.
+ bool isZeroInitializable() const { return zeroInitializable; }
+
+ /// Check whether this struct can be C++ zero-initialized
+ /// with a zeroinitializer when considered as a base subobject.
+ bool isZeroInitializableAsBase() const { return zeroInitializableAsBase; }
};
} // namespace clang::CIRGen
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 2b95d2e12014c..1e9d06593315c 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -77,6 +77,8 @@ struct CIRRecordLowering final {
return astContext.toCharUnitsFromBits(bitOffset);
}
+ void calculateZeroInit();
+
CharUnits getSize(mlir::Type Ty) {
return CharUnits::fromQuantity(dataLayout.layout.getTypeSize(Ty));
}
@@ -183,7 +185,7 @@ void CIRRecordLowering::lower() {
accumulateFields();
- if (auto const *cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordDecl)) {
+ if (const auto *cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordDecl)) {
if (cxxRecordDecl->getNumBases() > 0) {
CIRGenModule &cgm = cirGenTypes.getCGModule();
cgm.errorNYI(recordDecl->getSourceRange(),
@@ -244,6 +246,19 @@ void CIRRecordLowering::accumulateFields() {
}
}
+void CIRRecordLowering::calculateZeroInit() {
+ for (const MemberInfo &member : members) {
+ if (member.kind == MemberInfo::InfoKind::Field) {
+ if (!member.fieldDecl || isZeroInitializable(member.fieldDecl))
+ continue;
+ zeroInitializable = zeroInitializableAsBase = false;
+ return;
+ }
+ // TODO(cir): handle base types
+ assert(!cir::MissingFeatures::cxxSupport());
+ }
+}
+
void CIRRecordLowering::determinePacked() {
if (packed)
return;
@@ -315,7 +330,9 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd,
cir::RecordType *ty) {
assert(!cir::MissingFeatures::astRecordDeclAttr());
ty->complete(lowering.fieldTypes, lowering.packed, lowering.padded);
- auto rl = std::make_unique<CIRGenRecordLayout>(ty ? *ty : cir::RecordType());
+ auto rl = std::make_unique<CIRGenRecordLayout>(
+ ty ? *ty : cir::RecordType(), (bool)lowering.zeroInitializable,
+ (bool)lowering.zeroInitializableAsBase);
assert(!cir::MissingFeatures::recordZeroInit());
assert(!cir::MissingFeatures::cxxSupport());
diff --git a/clang/test/CIR/CodeGen/union.cpp b/clang/test/CIR/CodeGen/union.cpp
new file mode 100644
index 0000000000000..24cd93f6b8edb
--- /dev/null
+++ b/clang/test/CIR/CodeGen/union.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -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 -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: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+
+// Should generate a union type with all members preserved.
+union U {
+ bool b;
+ short s;
+ int i;
+ float f;
+ double d;
+};
+// CIR: !rec_U = !cir.record<union "U" {!cir.bool, !s16i, !s32i, !cir.float,
!cir.double}>
+// LLVM: %union.U = type { double }
+// OGCG: %union.U = type { double }
+
+void shouldGenerateUnionAccess(union U u) {
+ u.b = true;
+ u.b;
+ u.i = 1;
+ u.i;
+ u.f = 0.1F;
+ u.f;
+ u.d = 0.1;
+ u.d;
+}
+// CIR: cir.func {{.*}}shouldGenerateUnionAccess
+// CIR: %[[#BASE:]] = cir.get_member %0[0] {name = "b"} : !cir.ptr<!rec_U>
-> !cir.ptr<!cir.bool>
+// CIR: cir.store %{{.+}}, %[[#BASE]] : !cir.bool, !cir.ptr<!cir.bool>
+// CIR: cir.get_member %0[0] {name = "b"} : !cir.ptr<!rec_U> ->
!cir.ptr<!cir.bool>
+// CIR: %[[#BASE:]] = cir.get_member %0[2] {name = "i"} : !cir.ptr<!rec_U>
-> !cir.ptr<!s32i>
+// CIR: cir.store %{{.+}}, %[[#BASE]] : !s32i, !cir.ptr<!s32i>
+// CIR: %[[#BASE:]] = cir.get_member %0[2] {name = "i"} : !cir.ptr<!rec_U>
-> !cir.ptr<!s32i>
+// CIR: %[[#BASE:]] = cir.get_member %0[3] {name = "f"} : !cir.ptr<!rec_U>
-> !cir.ptr<!cir.float>
+// CIR: cir.store %{{.+}}, %[[#BASE]] : !cir.float, !cir.ptr<!cir.float>
+// CIR: %[[#BASE:]] = cir.get_member %0[3] {name = "f"} : !cir.ptr<!rec_U>
-> !cir.ptr<!cir.float>
+// CIR: %[[#BASE:]] = cir.get_member %0[4] {name = "d"} : !cir.ptr<!rec_U>
-> !cir.ptr<!cir.double>
+// CIR: cir.store %{{.+}}, %[[#BASE]] : !cir.double, !cir.ptr<!cir.double>
+// CIR: %[[#BASE:]] = cir.get_member %0[4] {name = "d"} : !cir.ptr<!rec_U>
-> !cir.ptr<!cir.double>
+
+// LLVM: define {{.*}}shouldGenerateUnionAccess
+// LLVM: %[[BASE:.*]] = alloca %union.U
+// LLVM: store %union.U %{{.*}}, ptr %[[BASE]]
+// LLVM: store i8 1, ptr %[[BASE]]
+// LLVM: store i32 1, ptr %[[BASE]]
+// LLVM: store float 0x3FB99999A0000000, ptr %[[BASE]]
+// LLVM: store double 1.000000e-01, ptr %[[BASE]]
+
+// OGCG: define {{.*}}shouldGenerateUnionAccess
+// OGCG: %[[BASE:.*]] = alloca %union.U
+// OGCG: %[[DIVE:.*]] = getelementptr inbounds nuw %union.U, ptr %[[BASE]],
i32 0, i32 0
+// OGCG: store i64 %{{.*}}, ptr %[[DIVE]]
+// OGCG: store i8 1, ptr %[[BASE]]
+// OGCG: store i32 1, ptr %[[BASE]]
+// OGCG: store float 0x3FB99999A0000000, ptr %[[BASE]]
+// OGCG: store double 1.000000e-01, ptr %[[BASE]]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
