On Sep 13, 2012, at 12:22 AM, Chandler Carruth wrote:
> With all that discussion, here is an updated patch. I've included most of the
> tricky test cases described here (where they seemed valuable) along with lots
> of comments to help document some of this discussion and the conclusions.
+ // See if there are other bits in the bitfield's storage we'll need to load
+ // and mask together with source before storing.
+ if (Info.StorageSize != Info.Size) {
+ assert(Info.StorageSize > Info.Size && "Invalid bitfield size.");
Info.StorageSize is always a multiple of the byte size, right? If a bitfield
is all alone in its storage unit, shouldn't we be able to avoid the
load/mask/or stuff?
+ // Mask out the original value.
+ Val = Builder.CreateAnd(Val,
+ ~llvm::APInt::getBitsSet(Info.StorageSize,
+ Info.Offset,
+ Info.Offset + Info.Size),
+ "bf.clear");
This masking is unnecessary if the value is known to be all-ones.
The backend can do this peephole pretty easily, but it's worth doing
ourselves for -O0 goodness.
+ // Or together the unchanged values and the source value.
+ SrcVal = Builder.CreateOr(Val, SrcVal, "bf.set");
And analogously, this is unnecessary if the value is known to be
all-zeroes.
+ /// The storage size in bits which should be used when accessing this
+ /// bitfield.
+ unsigned StorageSize;
This should document any invariants like "this is always a multiple of
the bit-width of a char".
- assert(!FD->isBitField() && "Invalid call for bit-field decl!");
Is removing this assertion important? It's a nice way to ensure that
code isn't naively accessing a bitfield like a normal field.
I'll admit to only skimming right now; if you want a full review,
feel free to wait longer. :)
John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
