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

andykaylor · 2025-05-03T00:18:40Z

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.

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.

llvmbot · 2025-05-03T00:19:12Z

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/138368.diff

5 Files Affected:

(modified) clang/lib/CIR/CodeGen/CIRGenExpr.cpp (+6-3)
(modified) clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp (+9-4)
(modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+18-7)
(modified) clang/lib/CIR/CodeGen/CIRGenTypes.cpp (+5-2)
(modified) clang/test/CIR/CodeGen/struct.cpp (+37)

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

llvmbot · 2025-05-03T00:19:12Z

@llvm/pr-subscribers-clangir

Author: Andy Kaylor (andykaylor)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/138368.diff

5 Files Affected:

(modified) clang/lib/CIR/CodeGen/CIRGenExpr.cpp (+6-3)
(modified) clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp (+9-4)
(modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+18-7)
(modified) clang/lib/CIR/CodeGen/CIRGenTypes.cpp (+5-2)
(modified) clang/test/CIR/CodeGen/struct.cpp (+37)

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

clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp

mmha

LGTM, just a few nits

mmha · 2025-05-03T15:15:55Z

clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp

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


OGCG calls emitNullConstant here. We have CIRGenModule::emitNullConstant partially implemented. In the incubator CIRGenModule::emitNullConstant fails with a NYi if the record type is not zero initializable and IMO that's a reasonable limitation to have for now (instead of silently breaking vtables and default ctors).

Yeah, that seems reasonable.

As I tried to implement this suggestion, it quickly came to my attention that emitNullConstant returns a value, and we need an attribute here. I think the cd->isTrivial() && cd->isDefaultConstructor() check should prevent us from getting here in any case where the zero attribute isn't sufficient, but I will add an assert that the type is zero-initializable.

Second complication, the clang AST says that CompleteS is not zero-initializable for the test case I'm adding in struct.cpp. (But classic codegen generates zeroinitializer for it). I think I may need to implement CIRGenRecordLayout::isZeroInitializableAsBase for this.

Thanks for adding the errorNYI for structs that are not "truly" zero initializable for a given ABI. LGTM now.

clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp

clang/lib/CIR/CodeGen/CIRGenTypes.cpp

clang/test/CIR/CodeGen/struct.cpp

erichkeane

I'm happy when morris is, else 1 small question.

erichkeane · 2025-05-05T13:24:31Z

clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp

-        return {};
+      if (const CXXConstructExpr *e =
+              dyn_cast_or_null<CXXConstructExpr>(d.getInit())) {
+        const CXXConstructorDecl *cd = e->getConstructor();


Does classic code-gen assume that cd is non-null here? I'm not positive it always is.

Also, do we/should we model elidable here?

Classic codegen does assume that cd is non-null here. I'm pretty sure that if the dyn_cast_or_null gives us a CXXConstructorExpr we can count on if having a CXXConstructorDecl.

The AST handles any decisions about which constructors are to be called. Currently, the incubator just represents constructors, when needed, as cir.call operations. It may be worth doing more than that. In the case of code like this:

void f() { MyClass obj = func(); }

The AST says this:

-FunctionDecl <line:12:1, line:14:1> line:12:6 f 'void ()' `-CompoundStmt <col:10, line:14:1> `-DeclStmt <line:13:5, col:25> `-VarDecl <col:5, col:24> col:13 obj 'MyClass' cinit `-CallExpr <col:19, col:24> 'MyClass' `-ImplicitCastExpr <col:19> 'MyClass (*)()' <FunctionToPointerDecay> `-DeclRefExpr <col:19> 'MyClass ()' lvalue Function 0x216ac150 'func' 'MyClass ()'

And the incubator generates this CIR:

cir.func @_Z1fv() { %0 = cir.alloca !rec_MyClass, !cir.ptr<!rec_MyClass>, ["obj", init] {alignment = 1 : i64} %1 = cir.call @_Z4funcv() : () -> !rec_MyClass cir.store %1, %0 : !rec_MyClass, !cir.ptr<!rec_MyClass> cir.return }

Is your question whether we should do something here to explicitly show that the copy constructor was elided?

Ok, glad to know. Sometimes the decl elements of an expr can be null in subtle situations.

As far as elidable. There is a CXXConstructExpr::isElidable that I'm surprised we don't have to express here, since the idea is that we could skip the construction.

I see. If I add -std=c++14 to my compile line, that gets the elidable constructor in the AST, and then -fno-elide-constructors get us to emit CIR for it but not here. That's handled in emitCXXConstructExpr (which has not been upstreamed yet). I guess the cd->isTrivial() && cd->isDefaultConstructor() check prevents us from doing anything with it here.

We have been careful not to elide things too early since it might hurt static analysis - this is a big pain point and request from the static analysis community (as clang really early get rid of these). The idea is to represent them in a way that during LoweringPrepare we can either elide them out or apply specific lowering heuristics (e.g. when deciding the best way to initialize large set of constant datas).

@bcardosolopes Does that mean that we should be emitting an elidable constructor in CIR even if the -fno-elide-constructors option isn't used? The incubator is eliding them the same way classic codegen does.

For the case without -std=c++14 I'm not sure we have much choice since it's not even in the AST.

bcardosolopes

LGTM

el-ev

LGTM

andykaylor requested review from bcardosolopes, mmha, erichkeane, lanza and el-ev May 3, 2025 00:18

llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels May 3, 2025

mmha reviewed May 3, 2025

View reviewed changes

clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp Outdated Show resolved Hide resolved

mmha approved these changes May 3, 2025

View reviewed changes

erichkeane approved these changes May 5, 2025

View reviewed changes

Add check for base class and test for unions

a160ec9

bcardosolopes approved these changes May 6, 2025

View reviewed changes

Add error and test for non-zero-init structs

84f01c9

el-ev approved these changes May 7, 2025

View reviewed changes

andykaylor merged commit 8d9f516 into llvm:main May 7, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

andykaylor commented May 3, 2025

llvmbot commented May 3, 2025

llvmbot commented May 3, 2025

mmha left a comment

mmha May 3, 2025

andykaylor May 5, 2025

andykaylor May 5, 2025

andykaylor May 5, 2025

mmha May 6, 2025

erichkeane left a comment

erichkeane May 5, 2025

andykaylor May 5, 2025

erichkeane May 5, 2025

andykaylor May 5, 2025

bcardosolopes May 6, 2025

andykaylor May 6, 2025

bcardosolopes left a comment

el-ev left a comment

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

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

Conversation

andykaylor commented May 3, 2025

llvmbot commented May 3, 2025

llvmbot commented May 3, 2025

mmha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erichkeane left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcardosolopes left a comment

Choose a reason for hiding this comment

el-ev left a comment

Choose a reason for hiding this comment