[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 llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 9 20:44:37 PDT 2017
On 08/09/2017 10:37 PM, Hal Finkel via llvm-commits wrote:
> On 08/09/2017 10:14 PM, Xinliang David Li via llvm-commits wrote:
>> On Wed, Aug 9, 2017 at 7:42 PM, Chandler Carruth via Phabricator
>> <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
>> chandlerc added a comment.
>> This has been discussed before and I still pretty strongly
>> disagree with it.
>> This cripples the ability of TSan to find race conditions between
>> accesses to consecutive bitfields -- and these bugs have actually
>> come up.
>> Can you elaborate on this? Do you mean race conditions due to widening?
>> We also have had cases in the past where LLVM missed significant
>> bitfield combines and optimizations due to loading them as
>> separate integers. Those would become problems again, and I think
>> they would be even harder to solve than narrowing the access is
>> going to be because we will have strictly less information to
>> work with.
>> Can you elaborate here too? If there were missed optimization that
>> later got fixed, there should be regression tests for them, right?
Also, on this, unfortunately, no. The problem is that the optimization
would be tested using a specific regression test. That test will
continue to pass even if you change the IR that frontend generates so
that the test no longer represents the partially-optimized IR pattern
that the frontend will help create. This is why our tendency to rely
only on pass-specific regression tests, and not having end-to-end tests,
is not necessarily optimal.
>> And what information is missing?
> To make a general statement, if we load (a, i8) and (a+2, i16), for
> example, and these came from some structure, we've lost the
> information that the load (a+1, i8) would have been legal (i.e. is
> known to be deferenceable). This is not specific to bit fields, but
> the fact that we lose information on the dereferenceable byte ranges
> around memory access turns into a problem when we later can't legally
> widen. There may be a better way to keep this information other than
> producing wide loads (which is an imperfect mechanism, especially the
> way we do it by restricting to legal integer types), but at the
> moment, we don't have anything better.
>> Ultimately, while I understand the appeal of this approach, I
>> don't think it is correct and I think we should instead figure
>> out how to optimize these memory accesses well in LLVM. That
>> approach will have the added benefit of optimizing cases where
>> the user has manually used a large integer to simulate bitfields,
>> and making combining and canonicalization substantially easier.
>> rL LLVM
>> https://reviews.llvm.org/D36562 <https://reviews.llvm.org/D36562>
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits