hfinkel added a comment.
You seem to be only changing the behavior for the "separatable" fields, but I
suspect you want to change the behavior for the others too. The bitfield would
be decomposed into shards, separated by the naturally-sized-and-aligned fields.
Each access only loads its shard. For example, in your test case you have:
struct S3 {
unsigned long f1:14;
unsigned long f2:18;
unsigned long f3:32;
};
and you test that, with this option, loading/storing to a3.f3 only access the
specific 4 bytes composing f3. But if you load f1 or f2, we're still loading
all 8 bytes, right? I think we should only load/store the lower 4 bytes when we
access a3.f1 and/or a3.f2.
Otherwise, you can again end up with the narrow-store/wide-load problem for
nearby fields under a different set of circumstances.
================
Comment at: include/clang/Driver/Options.td:1032
+def fsplit_bitfields : Flag<["-"], "fsplit-bitfields">,
+ Group<f_clang_Group>, Flags<[CC1Option]>,
----------------
I'm not opposed to -fsplit-bitfields, but I'd prefer if we find something more
self-explanatory. It's not really clear what "splitting a bitfield" means.
Maybe?
-fsplit-bitfield-accesses
-fdecomposed-bitfield-accesses
-fsharded-bitfield-accesses
-ffine-grained-bitfield-accesses
(I think that I prefer -ffine-grained-bitfield-accesses, although it's the
longest)
================
Comment at: include/clang/Driver/Options.td:1034
+ Group<f_clang_Group>, Flags<[CC1Option]>,
+ HelpText<"Enable splitting bitfield with legal width and alignment in
LLVM.">;
+def fno_split_bitfields : Flag<["-"], "fno-split-bitfields">,
----------------
How about?
Use separate access for bitfields with legal widths and alignments.
I don't think that "in LLVM" is needed here (or we could put "in LLVM" on an
awful lot of these options).
================
Comment at: lib/CodeGen/CGExpr.cpp:1679
+/// field has legal integer width, and its bitfield offset is naturally
+/// aligned, it is better to access the bitfield like a separate integer var.
+static bool IsSeparatableBitfield(const CGBitFieldInfo &Info,
----------------
var -> variable
Repository:
rL LLVM
https://reviews.llvm.org/D36562
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
