[PATCH] D139267: Supporting tbaa.struct metadata generation for bitfields

[Timo Stripf via Phabricator via cfe-commits]

strimo378 created this revision.
strimo378 added a project: clang.
Herald added subscribers: jeroen.dobbelaere, kosarev.
Herald added a project: All.
strimo378 requested review of this revision.
Herald added a subscriber: cfe-commits.

Hi all,

this is my first contribution to the LLVM project.

The patch adds support for generating the tbaa.struct metadata for structs containing bitfields. The current implementation does not support bitfields and even generates wrong tbaa.struct metadata ... see https://github.com/llvm/llvm-project/issues/59328 .

The patch is not yet finished. I need some help to create an appropriate test case. My first test case looks like that

  struct a {
    int : 8;
    int b1 : 4;
    int b2 : 8;
  };
  a i1,i2;
  void c() { i1 = i2; }

and should generate the following metadata

    call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 bitcast (%struct.a* @i1 to i8*), i8* align 4 bitcast (%struct.a* @i2 to i8*), i64 4, i1 false), !tbaa.struct !3
  ...
    !3 = !{i64 1, i64 2, !4}

Furthermore, I am unsure what Type should be used for adding bitfields to tbaa.struct. The original type or char? The current implementation uses the original type (if not alias).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139267

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/lib/CodeGen/CodeGenTBAA.h


Index: clang/lib/CodeGen/CodeGenTBAA.h
===================================================================
--- clang/lib/CodeGen/CodeGenTBAA.h
+++ clang/lib/CodeGen/CodeGenTBAA.h
@@ -146,6 +146,12 @@
   /// considered to be equivalent to it.
   llvm::MDNode *getChar();
 
+  /// AddCollectedField - Add one collected field to Fields vector
+  void
+  AddCollectedField(SmallVectorImpl<llvm::MDBuilder::TBAAStructField> &Fields,
+                    uint64_t Offset, uint64_t Size, QualType QTy, bool MayAlias,
+                    bool FuseOverlapping);
+
   /// CollectFields - Collect information about the fields of a type for
   /// !tbaa.struct metadata formation. Return false for an unsupported type.
   bool CollectFields(uint64_t BaseOffset,
Index: clang/lib/CodeGen/CodeGenTBAA.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenTBAA.cpp
+++ clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -272,13 +272,28 @@
                         Size);
 }
 
+void CodeGenTBAA::AddCollectedField(
+    SmallVectorImpl<llvm::MDBuilder::TBAAStructField> &Fields, uint64_t Offset,
+    uint64_t Size, QualType QTy, bool MayAlias, bool FuseOverlapping) {
+
+  // Fuse fields that overlap in their byte position (e.g. caused by bitfields)
+  if (FuseOverlapping && !Fields.empty() && Offset < Fields.back().Offset + Fields.back().Size) {
+    Fields.back().Size = Fields.back().Offset - Offset + Size;
+    return;
+  }
+
+  llvm::MDNode *TBAAType = MayAlias ? getChar() : getTypeInfo(QTy);
+  llvm::MDNode *TBAATag = getAccessTagInfo(TBAAAccessInfo(TBAAType, Size));
+  Fields.push_back(llvm::MDBuilder::TBAAStructField(Offset, Size, TBAATag));
+}
+
 bool
 CodeGenTBAA::CollectFields(uint64_t BaseOffset,
                            QualType QTy,
                            SmallVectorImpl<llvm::MDBuilder::TBAAStructField> &
                              Fields,
                            bool MayAlias) {
-  /* Things not handled yet include: C++ base classes, bitfields, */
+  /* Things not handled yet include: C++ base classes */
 
   if (const RecordType *TTy = QTy->getAs<RecordType>()) {
     const RecordDecl *RD = TTy->getDecl()->getDefinition();
@@ -300,19 +315,25 @@
       uint64_t Offset = BaseOffset +
                         Layout.getFieldOffset(idx) / Context.getCharWidth();
       QualType FieldQTy = i->getType();
-      if (!CollectFields(Offset, FieldQTy, Fields,
-                         MayAlias || TypeHasMayAlias(FieldQTy)))
-        return false;
+      bool FieldMayAlias = MayAlias || TypeHasMayAlias(FieldQTy);
+      if ((*i)->isBitField()) {
+        uint64_t EndOffset =
+            BaseOffset +
+            (Layout.getFieldOffset(idx) + (*i)->getBitWidthValue(Context) - 1) /
+                Context.getCharWidth();
+
+        AddCollectedField(Fields, Offset, EndOffset - Offset + 1, QTy, FieldMayAlias, true);
+      } else {
+        if (!CollectFields(Offset, FieldQTy, Fields, FieldMayAlias))
+          return false;
+      }
     }
     return true;
   }
 
   /* Otherwise, treat whatever it is as a field. */
-  uint64_t Offset = BaseOffset;
   uint64_t Size = Context.getTypeSizeInChars(QTy).getQuantity();
-  llvm::MDNode *TBAAType = MayAlias ? getChar() : getTypeInfo(QTy);
-  llvm::MDNode *TBAATag = getAccessTagInfo(TBAAAccessInfo(TBAAType, Size));
-  Fields.push_back(llvm::MDBuilder::TBAAStructField(Offset, Size, TBAATag));
+  AddCollectedField(Fields, BaseOffset, Size, QTy, MayAlias, false);
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139267.479891.patch
Type: text/x-patch
Size: 3506 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221204/345e1fff/attachment-0001.bin>