[CIR] Unblock simple C++ structure support (#138368)

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.

Author: andykaylor

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();

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"
@@ -365,12 +366,33 @@ 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 (d.getInit() && isa<CXXConstructExpr>(d.getInit())) {
-        cgm.errorNYI(d.getInit()->getBeginLoc(),
-                     "tryEmitPrivateForVarInit CXXConstructExpr");
-        return {};
+    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()) {
+          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 {};
+          }
+          if (!cgm.getTypes().isZeroInitializable(cxxrd)) {
+            // To handle this case, we really need to go through
+            // emitNullConstant, but we need an attribute, not a value
+            cgm.errorNYI(
+                "tryEmitPrivateForVarInit: non-zero-initializable cxx record");
+            return {};
+          }
+          return cir::ZeroAttr::get(cgm.convertType(d.getType()));
+        }
       }
+    }
   }
   inConstantContext = d.hasConstantInitialization();
 

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

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));
   }
@@ -177,18 +179,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 (const auto *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());
@@ -199,6 +209,7 @@ void CIRRecordLowering::lower() {
   insertPadding();
   members.pop_back();
 
+  calculateZeroInit();
   fillOutputFields();
 }
 
@@ -236,6 +247,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;
@@ -295,7 +319,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
@@ -304,7 +331,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());

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.
@@ -497,9 +500,9 @@ bool CIRGenTypes::isZeroInitializable(clang::QualType t) {
         return true;
   }
 
-  if (t->getAs<RecordType>()) {
-    cgm.errorNYI(SourceLocation(), "isZeroInitializable for RecordType", t);
-    return false;
+  if (const RecordType *rt = t->getAs<RecordType>()) {
+    const RecordDecl *rd = rt->getDecl();
+    return isZeroInitializable(rd);
   }
 
   if (t->getAs<MemberPointerType>()) {
@@ -511,6 +514,10 @@ bool CIRGenTypes::isZeroInitializable(clang::QualType t) {
   return true;
 }
 
+bool CIRGenTypes::isZeroInitializable(const RecordDecl *rd) {
+  return getCIRGenRecordLayout(rd).isZeroInitializable();
+}
+
 const CIRGenFunctionInfo &CIRGenTypes::arrangeCIRFunctionInfo(
     CanQualType returnType, llvm::ArrayRef<clang::CanQualType> argTypes) {
   assert(llvm::all_of(argTypes,

clang/lib/CIR/CodeGen/CIRGenTypes.h

@@ -120,6 +120,7 @@ class CIRGenTypes {
   /// Return whether a type can be zero-initialized (in the C++ sense) with an
   /// LLVM zeroinitializer.
   bool isZeroInitializable(clang::QualType ty);
+  bool isZeroInitializable(const RecordDecl *rd);
 
   const CIRGenFunctionInfo &arrangeFreeFunctionCall(const CallArgList &args,
                                                     const FunctionType *fnType);

clang/test/CIR/CodeGen/nonzeroinit-struct.cpp

@@ -0,0 +1,19 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - 2>&1 | FileCheck %s
+
+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;
+
+// Since the case above isn't handled yet, we want a test that verifies that
+// we're failing for the right reason.
+
+// CHECK: error: ClangIR code gen Not Yet Implemented: tryEmitPrivateForVarInit: non-zero-initializable cxx record

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]]

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]]