[PATCH] D36562: [Bitfield] Make the bitfield a separate location if it has width of legal integer type and its bit offset is naturally aligned for the type

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:335
+def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
+  "option '-ffine-grained-bitfield-accesses' cannot be enabled together with sanitizer; flag ignored">,
+  InGroup<OptionIgnored>;
----------------
with a sanitizer


================
Comment at: include/clang/Driver/Options.td:1041
+  "ffine-grained-bitfield-accesses">, Group<f_clang_Group>, Flags<[CC1Option]>,
+  HelpText<"Use separate access for bitfields with legal widths and alignments.">;
+def fno_fine_grained_bitfield_accesses : Flag<["-"],
----------------
access -> accesses


================
Comment at: include/clang/Driver/Options.td:1044
+  "fno-fine-grained-bitfield-accesses">, Group<f_clang_Group>, Flags<[CC1Option]>,
+  HelpText<"Use a big integer wrap for a consecutive run of bitfields.">;
+
----------------
Use large-integer access for consecutive bitfield runs.


================
Comment at: include/clang/Frontend/CodeGenOptions.def:182
 CODEGENOPT(SoftFloat         , 1, 0) ///< -soft-float.
+CODEGENOPT(FineGrainedBitfieldAccesses, 1, 0) ///< Use separate access for bitfields
+                                              ///< with legal widths and alignments.
----------------
These lines are too long.


================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:449
+    // Otherwise, try to add bitfields to the run.
+    if (Run != FieldEnd && !IsBetterAsSingleFieldRun(Run) &&
+        Field != FieldEnd && !IsBetterAsSingleFieldRun(Field) &&
----------------
Why do you have the `IsBetterAsSingleFieldRun(Run)` check here (where we'll evaluate it multiple times (for all of the fields in the run)). Can't you make the predicate above directly?

      // Any non-zero-length bitfield can start a new run.
      if (Field->getBitWidthValue(Context) != 0 &&
           !IsBetterAsSingleFieldRun(Field)) {
        Run = Field;
        StartBitOffset = getFieldBitOffset(*Field);
        ...



Repository:
  rL LLVM

https://reviews.llvm.org/D36562