[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

[Strahinja Petrovic via Phabricator via cfe-commits]

spetrovic created this revision.
Herald added subscribers: arichardson, sdardis.

This patch provides that bitfields are splitted even in case when current field is not legal integer type. For Example,

struct S3 {

  unsigned long a1:28;
  unsigned long a2:4;
  unsigned long a3:12;

};

struct S4 {

  unsigned long long b1:61;
  unsigned long b2:3;
  unsigned long b3:3;

};

At the moment, S3 is i48 type, and S4 is i72 type, with this change S3 is treated as i32 + 16, and S4 is treated as i32 + i32 + i8.
With this patch we avoid unaligned loads and stores, at least on MIPS architecture.


https://reviews.llvm.org/D39053

Files:
  lib/CodeGen/CGRecordLayoutBuilder.cpp
  test/CodeGenCXX/finegrain-bitfield-type.cpp


Index: test/CodeGenCXX/finegrain-bitfield-type.cpp
===================================================================
--- test/CodeGenCXX/finegrain-bitfield-type.cpp
+++ test/CodeGenCXX/finegrain-bitfield-type.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffine-grained-bitfield-accesses \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+struct S4 {
+  unsigned long f1:28;
+  unsigned long f2:4;
+  unsigned long f3:12;
+};
+struct S4 a4;
+
+struct S5 {
+  unsigned long f1:28;
+  unsigned long f2:4;
+  unsigned long f3:28;
+  unsigned long f4:4;
+  unsigned long f5:12;
+};
+struct S5 a5;
+
+
+// CHECK: %struct.S4 = type { i32, i16 }
+// CHECK-NOT: %struct.S4 = type { i48 }
+// CHECK: %struct.S5 = type { i32, i32, i16, [6 x i8] }
+// CHECK-NOT: %struct.S5 = type { i80 }
\ No newline at end of file
Index: lib/CodeGen/CGRecordLayoutBuilder.cpp
===================================================================
--- lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -408,15 +408,15 @@
   // has legal integer width, and its bitfield offset is naturally aligned, it
   // is better to make the bitfield a separate storage component so as it can be
   // accessed directly with lower cost.
-  auto IsBetterAsSingleFieldRun = [&](RecordDecl::field_iterator Field) {
+  auto IsBetterAsSingleFieldRun = [&](uint64_t OffsetInRecord,
+                                      uint64_t StartBitOffset) {
     if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses)
       return false;
-    unsigned Width = Field->getBitWidthValue(Context);
-    if (!DataLayout.isLegalInteger(Width))
+    if (!DataLayout.isLegalInteger(OffsetInRecord))
       return false;
     // Make sure Field is natually aligned if it is treated as an IType integer.
-    if (getFieldBitOffset(*Field) %
-            Context.toBits(getAlignment(getIntNType(Width))) !=
+    if (StartBitOffset %
+            Context.toBits(getAlignment(getIntNType(OffsetInRecord))) !=
         0)
       return false;
     return true;
@@ -435,7 +435,8 @@
         Run = Field;
         StartBitOffset = getFieldBitOffset(*Field);
         Tail = StartBitOffset + Field->getBitWidthValue(Context);
-        StartFieldAsSingleRun = IsBetterAsSingleFieldRun(Run);
+        StartFieldAsSingleRun = IsBetterAsSingleFieldRun(Tail - StartBitOffset,
+                                                         StartBitOffset);
       }
       ++Field;
       continue;
@@ -449,7 +450,7 @@
     // skip the block below and go ahead to emit the storage.
     // Otherwise, try to add bitfields to the run.
     if (!StartFieldAsSingleRun && Field != FieldEnd &&
-        !IsBetterAsSingleFieldRun(Field) &&
+        !IsBetterAsSingleFieldRun(Tail - StartBitOffset, StartBitOffset) &&
         Field->getBitWidthValue(Context) != 0 &&
         Tail == getFieldBitOffset(*Field)) {
       Tail += Field->getBitWidthValue(Context);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39053.119468.patch
Type: text/x-patch
Size: 2926 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171018/50677417/attachment.bin>