[cfe-commits] PATCH: Large re-working of bitfield IR-gen, and a fix for PR13691

[Chandler Carruth]

Updated patch.

Now passes all regression tests. Tests for the PR in question have been
added.

I've implemented some really gross logic in the ObjCRuntime part of IR-Gen.
Hope it's OK... =/

I'd love to get some feedback on this and commit it soon. I'll be
bootstrapping and running it through the test suite when I can.


On Mon, Aug 27, 2012 at 2:17 AM, Chandler Carruth <chandlerc at google.com>wrote:

> Hello,
>
> I've attached a *draft* patch that is a re-working of our record layout
> and IR generation for bitfields. The previous strategy inherently
> introduced widened stores (and loads) that violate the C++11 memory model,
> causing PR13691. The old system was also rather elaborate without much
> justification. My patch shrinks the code significantly.
>
> LLVM supports arbitrary sized integers, and bitfields *really are* the
> equivalent of a bag-of-bits or integer-vector that these integers support
> well. SRoA has been forming such integers in LLVM for some time. The
> various targets are in a good position to split and simplify these
> operations into the efficient representation... and when they fail, we
> should fix the target. So this patch essentially interpets a run of
> contiguous bitfields as a single large integer, and emits loads stores and
> bit operations appropriate for extracting bits from within it. This leaves
> any valid load widening to LLVM passes that are more target aware and also
> aware of instrumentation such as ASan or TSan that might be invalidated by
> load widening.
>
> This has a few advantages:
> - Very dense IR, we can now pre-compute the GEP when building the LValue
> for a bitfield, and that will be the only GEP needed. Also, the math to
> extract the value is simpler with this model.
> - Maximal exposure to the optimizer of coalescing opportunities across
> bitfields. The SRoA of the IR produces will end up with a single SSA value
> for the collection of bits in the bitfield.
>
> It also has a two specific disadvantages in its current form, but I think
> I can address them eventually:
> - It doesn't take advantage of unused bits, which can be assumed to be
> all-zero and save some masking.
> - It also doesn't take advantage of padding following the bitfield to
> widen stores safely
>
> The last one is very tricky to do correctly. We have to know that the
> object is POD-for-layout. I want to address this separately w/ some good
> test cases.
>
>
> I hope to send up a cleaned up patch with test cases and results from a
> bootstrap and maybe test suite run tomorrow, but wanted to give folks an
> opportunity to comment on the basic strategy.
>
> Somewhat terrifyingly, *all current regression tests pass*. =/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120827/5dd011dc/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PR13691.patch
Type: application/octet-stream
Size: 57408 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120827/5dd011dc/attachment.obj>