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

[Strahinja Petrovic via Phabricator via cfe-commits]

spetrovic updated this revision to Diff 143918.
spetrovic added a comment.

Comments addressed


https://reviews.llvm.org/D39053

Files:
  include/clang/Driver/Options.td
  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,22 @@
+// 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
@@ -404,19 +404,20 @@
     return;
   }
 
-  // Check if current Field is better as a single field run. When current field
+  // Check if OffsetInRecord is better as a single field run. When OffsetInRecord
   // 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))) !=
+    // Make sure StartBitOffset is natually aligned if it is treated as an
+    // IType integer.
+     if (StartBitOffset %
+            Context.toBits(getAlignment(getIntNType(OffsetInRecord))) !=
         0)
       return false;
     return true;
@@ -435,23 +436,24 @@
         Run = Field;
         StartBitOffset = getFieldBitOffset(*Field);
         Tail = StartBitOffset + Field->getBitWidthValue(Context);
-        StartFieldAsSingleRun = IsBetterAsSingleFieldRun(Run);
+        StartFieldAsSingleRun = IsBetterAsSingleFieldRun(Tail - StartBitOffset,
+                                                         StartBitOffset); 
       }
       ++Field;
       continue;
     }
 
     // If the start field of a new run is better as a single run, or
-    // if current field is better as a single run, or
+    // if current field (or consecutive fields) is better as a single run, or
     // if current field has zero width bitfield and either
     // UseZeroLengthBitfieldAlignment or UseBitFieldTypeAlignment is set to
     // true, or
     // if the offset of current field is inconsistent with the offset of
     // previous field plus its offset,
     // 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->isZeroLengthBitField(Context) ||
          (!Context.getTargetInfo().useZeroLengthBitfieldAlignment() &&
           !Context.getTargetInfo().useBitFieldTypeAlignment())) &&
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1138,7 +1138,7 @@
 
 def ffine_grained_bitfield_accesses : Flag<["-"],
   "ffine-grained-bitfield-accesses">, Group<f_clang_Group>, Flags<[CC1Option]>,
-  HelpText<"Use separate accesses for bitfields with legal widths and alignments.">;
+  HelpText<"Use separate accesses for consecutive bitfield runs with legal widths and alignments.">;
 def fno_fine_grained_bitfield_accesses : Flag<["-"],
   "fno-fine-grained-bitfield-accesses">, Group<f_clang_Group>, Flags<[CC1Option]>,
   HelpText<"Use large-integer access for consecutive bitfield runs.">;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits