[CIR][NFC] Cleanup MissingFeature asserts in RecordLayoutBuilder

[andykaylor]

This change cleans up some cir::MissingFeature asserts in CIRGenRecordLayoutBuilder.cpp. In a couple of cases the asserts were stale markers that we failed to remove when the correspond support was implemented.

In one case, a cir::MissingFeature::bitfields() assert was ambiguous in meaning and has been replaced by a comment and something more specific. The missing feature in this case is just a bit of debug code to verify certain bitfield-related conditions. This check should be added, but it is not functionally required.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Andy Kaylor (andykaylor)

Changes

This change cleans up some cir::MissingFeature asserts in CIRGenRecordLayoutBuilder.cpp. In a couple of cases the asserts were stale markers that we failed to remove when the correspond support was implemented.

In one case, a cir::MissingFeature::bitfields() assert was ambiguous in meaning and has been replaced by a comment and something more specific. The missing feature in this case is just a bit of debug code to verify certain bitfield-related conditions. This check should be added, but it is not functionally required.

---

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

2 Files Affected:

• (modified) clang/include/clang/CIR/MissingFeatures.h (+1-1)
• (modified) clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp (+6-9)

diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 7a6c084f51cd7..3dfcafc0399a5 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -133,7 +133,6 @@ struct MissingFeatures {
   // RecordType
   static bool skippedLayout() { return false; }
   static bool astRecordDeclAttr() { return false; }
-  static bool recordZeroInit() { return false; }
   static bool recordZeroInitPadding() { return false; }
   static bool zeroSizeRecordMembers() { return false; }
 
@@ -192,6 +191,7 @@ struct MissingFeatures {
   static bool builtinCheckKind() { return false; }
   static bool cgCapturedStmtInfo() { return false; }
   static bool cgFPOptionsRAII() { return false; }
+  static bool checkBitfieldClipping() { return false; }
   static bool cirgenABIInfo() { return false; }
   static bool cleanupAfterErrorDiags() { return false; }
   static bool cleanupsToDeactivate() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
index 2baeb43c48b2e..87f23409e8e4b 100644
--- a/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenRecordLayoutBuilder.cpp
@@ -296,9 +296,8 @@ void CIRRecordLowering::lower(bool nonVirtualBaseType) {
   }
 
   llvm::stable_sort(members);
-  // TODO: implement clipTailPadding once bitfields are implemented
-  assert(!cir::MissingFeatures::bitfields());
-  assert(!cir::MissingFeatures::recordZeroInit());
+  // TODO: Verify bitfield clipping
+  assert(!cir::MissingFeatures::checkBitfieldClipping());
 
   members.push_back(makeStorageInfo(size, getUIntNType(8)));
   determinePacked(nonVirtualBaseType);
@@ -319,9 +318,11 @@ void CIRRecordLowering::fillOutputFields() {
         fieldIdxMap[member.fieldDecl->getCanonicalDecl()] =
             fieldTypes.size() - 1;
       // A field without storage must be a bitfield.
-      assert(!cir::MissingFeatures::bitfields());
-      if (!member.data)
+      if (!member.data) {
+        assert(member.fieldDecl &&
+               "member.data is a nullptr so member.fieldDecl should not be");
         setBitFieldInfo(member.fieldDecl, member.offset, fieldTypes.back());
+      }
     } else if (member.kind == MemberInfo::InfoKind::Base) {
       nonVirtualBases[member.cxxRecordDecl] = fieldTypes.size() - 1;
     } else if (member.kind == MemberInfo::InfoKind::VBase) {
@@ -697,13 +698,9 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *rd, cir::RecordType *ty) {
       ty ? *ty : cir::RecordType{}, baseTy ? baseTy : cir::RecordType{},
       (bool)lowering.zeroInitializable, (bool)lowering.zeroInitializableAsBase);
 
-  assert(!cir::MissingFeatures::recordZeroInit());
-
   rl->nonVirtualBases.swap(lowering.nonVirtualBases);
   rl->completeObjectVirtualBases.swap(lowering.virtualBases);
 
-  assert(!cir::MissingFeatures::bitfields());
-
   // Add all the field numbers.
   rl->fieldIdxMap.swap(lowering.fieldIdxMap);
 

[Andres-Salamanca]

Thanks for marking this I’ll prepare a PR to add support for this.

[andykaylor]

@Andres-Salamanca There are a couple of other cases in the constant ILE handling where bitfield support is needed, which is what led me to consider the markers here. The ILE code was added recently by @mmha and he may have an implementation prepared. He's on vacation this week, but I'll talk to him about it when he gets back.

[Andres-Salamanca]

Thanks for letting me know, I’ll wait for the follow-up.