[CIR] Implement zero-initialization of padding in constant records

[mmha]

This adds the zero initialization as required by the C standard.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Morris Hafner (mmha)

Changes

This adds the zero initialization as required by the C standard.

---

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

3 Files Affected:

• (modified) clang/include/clang/CIR/MissingFeatures.h (-1)
• (modified) clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp (+71-8)
• (added) clang/test/CIR/CodeGen/record-zero-init-padding.c (+90)

diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index f79580083cf9f..b0b825707aa7a 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -138,7 +138,6 @@ struct MissingFeatures {
   // RecordType
   static bool skippedLayout() { return false; }
   static bool astRecordDeclAttr() { return false; }
-  static bool recordZeroInitPadding() { return false; }
   static bool zeroSizeRecordMembers() { return false; }
 
   // Coroutines
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index 59aa25726a749..f577e23d7093e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -500,6 +500,26 @@ class ConstRecordBuilder {
   bool appendBitField(const FieldDecl *field, uint64_t fieldOffset,
                       cir::IntAttr ci, bool allowOverwrite = false);
 
+  /// Applies zero-initialization to padding bytes before and within a field.
+  /// \param layout The record layout containing field offset information.
+  /// \param fieldNo The field index in the record.
+  /// \param field The field declaration.
+  /// \param allowOverwrite Whether to allow overwriting existing values.
+  /// \param sizeSoFar The current size processed, updated by this function.
+  /// \param zeroFieldSize Set to true if the field has zero size.
+  /// \returns true on success, false if padding could not be applied.
+  bool applyZeroInitPadding(const ASTRecordLayout &layout, unsigned fieldNo,
+                            const FieldDecl &field, bool allowOverwrite,
+                            CharUnits &sizeSoFar, bool &zeroFieldSize);
+
+  /// Applies zero-initialization to trailing padding bytes in a record.
+  /// \param layout The record layout containing size information.
+  /// \param allowOverwrite Whether to allow overwriting existing values.
+  /// \param sizeSoFar The current size processed.
+  /// \returns true on success, false if padding could not be applied.
+  bool applyZeroInitPadding(const ASTRecordLayout &layout, bool allowOverwrite,
+                            CharUnits &sizeSoFar);
+
   bool build(InitListExpr *ile, bool allowOverwrite);
   bool build(const APValue &val, const RecordDecl *rd, bool isPrimaryBase,
              const CXXRecordDecl *vTableClass, CharUnits baseOffset);
@@ -548,6 +568,48 @@ bool ConstRecordBuilder::appendBitField(const FieldDecl *field,
                          allowOverwrite);
 }
 
+bool ConstRecordBuilder::applyZeroInitPadding(
+    const ASTRecordLayout &layout, unsigned fieldNo, const FieldDecl &field,
+    bool allowOverwrite, CharUnits &sizeSoFar, bool &zeroFieldSize) {
+  uint64_t startBitOffset = layout.getFieldOffset(fieldNo);
+  CharUnits startOffset =
+      cgm.getASTContext().toCharUnitsFromBits(startBitOffset);
+  if (sizeSoFar < startOffset) {
+    if (!appendBytes(sizeSoFar, computePadding(cgm, startOffset - sizeSoFar),
+                     allowOverwrite))
+      return false;
+  }
+
+  if (!field.isBitField()) {
+    CharUnits fieldSize =
+        cgm.getASTContext().getTypeSizeInChars(field.getType());
+    sizeSoFar = startOffset + fieldSize;
+    zeroFieldSize = fieldSize.isZero();
+  } else {
+    const CIRGenRecordLayout &rl =
+        cgm.getTypes().getCIRGenRecordLayout(field.getParent());
+    const CIRGenBitFieldInfo &info = rl.getBitFieldInfo(&field);
+    uint64_t endBitOffset = startBitOffset + info.size;
+    sizeSoFar = cgm.getASTContext().toCharUnitsFromBits(endBitOffset);
+    if (endBitOffset % cgm.getASTContext().getCharWidth() != 0)
+      sizeSoFar++;
+    zeroFieldSize = info.size == 0;
+  }
+  return true;
+}
+
+bool ConstRecordBuilder::applyZeroInitPadding(const ASTRecordLayout &layout,
+                                              bool allowOverwrite,
+                                              CharUnits &sizeSoFar) {
+  CharUnits totalSize = layout.getSize();
+  if (sizeSoFar < totalSize) {
+    if (!appendBytes(sizeSoFar, computePadding(cgm, totalSize - sizeSoFar),
+                     allowOverwrite))
+      return false;
+  }
+  return true;
+}
+
 bool ConstRecordBuilder::build(InitListExpr *ile, bool allowOverwrite) {
   RecordDecl *rd = ile->getType()
                        ->castAs<clang::RecordType>()
@@ -562,11 +624,9 @@ bool ConstRecordBuilder::build(InitListExpr *ile, bool allowOverwrite) {
     if (cxxrd->getNumBases())
       return false;
 
-  if (cgm.shouldZeroInitPadding()) {
-    assert(!cir::MissingFeatures::recordZeroInitPadding());
-    cgm.errorNYI(rd->getSourceRange(), "zero init padding");
-    return false;
-  }
+  const bool zeroInitPadding = cgm.shouldZeroInitPadding();
+  bool zeroFieldSize = false;
+  CharUnits sizeSoFar = CharUnits::Zero();
 
   unsigned elementNo = 0;
   for (auto [index, field] : llvm::enumerate(rd->fields())) {
@@ -596,7 +656,10 @@ bool ConstRecordBuilder::build(InitListExpr *ile, bool allowOverwrite) {
       continue;
     }
 
-    assert(!cir::MissingFeatures::recordZeroInitPadding());
+    if (zeroInitPadding &&
+        !applyZeroInitPadding(layout, index, *field, allowOverwrite, sizeSoFar,
+                              zeroFieldSize))
+      return false;
 
     // When emitting a DesignatedInitUpdateExpr, a nested InitListExpr
     // represents additional overwriting of our current constant value, and not
@@ -641,8 +704,8 @@ bool ConstRecordBuilder::build(InitListExpr *ile, bool allowOverwrite) {
     }
   }
 
-  assert(!cir::MissingFeatures::recordZeroInitPadding());
-  return true;
+  return !zeroInitPadding ||
+         applyZeroInitPadding(layout, allowOverwrite, sizeSoFar);
 }
 
 namespace {
diff --git a/clang/test/CIR/CodeGen/record-zero-init-padding.c b/clang/test/CIR/CodeGen/record-zero-init-padding.c
new file mode 100644
index 0000000000000..240dabba3c5d9
--- /dev/null
+++ b/clang/test/CIR/CodeGen/record-zero-init-padding.c
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
+
+struct padding_after_field {
+    char c;
+    int i;
+};
+
+struct bitfield_with_padding {
+    unsigned int a : 3;
+    unsigned int b : 5;
+    int c;
+};
+
+struct tail_padding {
+    int a;
+    char b;
+};
+
+struct multiple_padding {
+    char a;
+    short b;
+    long long c;
+};
+
+void test_zero_init_padding(void) {
+    static const struct padding_after_field paf = {1, 42};
+    static const struct bitfield_with_padding bfp = {1, 2, 99};
+    static const struct tail_padding tp = {10, 20};
+    static const struct multiple_padding mp = {5, 10, 100};
+}
+
+// Type definitions for anonymous structs with padding
+// CIR-DAG: !rec_anon_struct = !cir.record<struct  {!s8i, !u8i, !s16i, !cir.array<!u8i x 4>, !s64i}>
+// CIR-DAG: !rec_anon_struct1 = !cir.record<struct  {!s32i, !s8i, !cir.array<!u8i x 3>}>
+// CIR-DAG: !rec_anon_struct2 = !cir.record<struct  {!u8i, !cir.array<!u8i x 3>, !s32i}>
+// CIR-DAG: !rec_anon_struct3 = !cir.record<struct  {!s8i, !cir.array<!u8i x 3>, !s32i}>
+
+// paf: char + 3 bytes padding + int -> uses !rec_anon_struct3
+// CIR-DAG: cir.global "private" internal dso_local @test_zero_init_padding.paf = #cir.const_record<{
+// CIR-DAG-SAME:   #cir.int<1> : !s8i,
+// CIR-DAG-SAME:   #cir.const_array<[#cir.zero : !u8i, #cir.zero : !u8i, #cir.zero : !u8i]> : !cir.array<!u8i x 3>,
+// CIR-DAG-SAME:   #cir.int<42> : !s32i
+// CIR-DAG-SAME: }> : !rec_anon_struct3
+
+// bfp: unsigned bitfield byte + 3 bytes padding + int -> uses !rec_anon_struct2
+// CIR-DAG: cir.global "private" internal dso_local @test_zero_init_padding.bfp = #cir.const_record<{
+// CIR-DAG-SAME:   #cir.int<17> : !u8i,
+// CIR-DAG-SAME:   #cir.const_array<[#cir.zero : !u8i, #cir.zero : !u8i, #cir.zero : !u8i]> : !cir.array<!u8i x 3>,
+// CIR-DAG-SAME:   #cir.int<99> : !s32i
+// CIR-DAG-SAME: }> : !rec_anon_struct2
+
+// tp: int + char + 3 bytes tail padding -> uses !rec_anon_struct1
+// CIR-DAG: cir.global "private" internal dso_local @test_zero_init_padding.tp = #cir.const_record<{
+// CIR-DAG-SAME:   #cir.int<10> : !s32i,
+// CIR-DAG-SAME:   #cir.int<20> : !s8i,
+// CIR-DAG-SAME:   #cir.const_array<[#cir.zero : !u8i, #cir.zero : !u8i, #cir.zero : !u8i]> : !cir.array<!u8i x 3>
+// CIR-DAG-SAME: }> : !rec_anon_struct1
+
+// mp: char + 1 byte padding + short + 4 bytes padding + long long -> uses !rec_anon_struct
+// CIR-DAG: cir.global "private" internal dso_local @test_zero_init_padding.mp = #cir.const_record<{
+// CIR-DAG-SAME:   #cir.int<5> : !s8i,
+// CIR-DAG-SAME:   #cir.zero : !u8i,
+// CIR-DAG-SAME:   #cir.int<10> : !s16i,
+// CIR-DAG-SAME:   #cir.const_array<[#cir.zero : !u8i, #cir.zero : !u8i, #cir.zero : !u8i, #cir.zero : !u8i]> : !cir.array<!u8i x 4>,
+// CIR-DAG-SAME:   #cir.int<100> : !s64i
+// CIR-DAG-SAME: }> : !rec_anon_struct
+
+// CIR-LABEL: cir.func {{.*}}@test_zero_init_padding
+// CIR:   cir.return
+
+// LLVM-DAG: @test_zero_init_padding.paf = internal global { i8, [3 x i8], i32 } { i8 1, [3 x i8] zeroinitializer, i32 42 }
+// LLVM-DAG: @test_zero_init_padding.bfp = internal global { i8, [3 x i8], i32 } { i8 17, [3 x i8] zeroinitializer, i32 99 }
+// LLVM-DAG: @test_zero_init_padding.tp = internal global { i32, i8, [3 x i8] } { i32 10, i8 20, [3 x i8] zeroinitializer }
+// LLVM-DAG: @test_zero_init_padding.mp = internal global { i8, i8, i16, [4 x i8], i64 } { i8 5, i8 0, i16 10, [4 x i8] zeroinitializer, i64 100 }
+
+// LLVM-LABEL: define{{.*}} void @test_zero_init_padding
+// LLVM:   ret void
+
+// OGCG-DAG: @test_zero_init_padding.paf = internal constant { i8, [3 x i8], i32 } { i8 1, [3 x i8] zeroinitializer, i32 42 }
+// OGCG-DAG: @test_zero_init_padding.bfp = internal constant { i8, [3 x i8], i32 } { i8 17, [3 x i8] zeroinitializer, i32 99 }
+// OGCG-DAG: @test_zero_init_padding.tp = internal constant { i32, i8, [3 x i8] } { i32 10, i8 20, [3 x i8] zeroinitializer }
+// OGCG-DAG: @test_zero_init_padding.mp = internal constant { i8, i8, i16, [4 x i8], i64 } { i8 5, i8 0, i16 10, [4 x i8] zeroinitializer, i64 100 }
+
+// OGCG-LABEL: define{{.*}} void @test_zero_init_padding
+// OGCG:   ret void

[andykaylor]

lgtm

[bcardosolopes]

LGTM, one super nit

[bcardosolopes]

two space indentation