[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

[Xinliang David Li via llvm-commits]

On Wed, Aug 9, 2017 at 8:44 PM, Hal Finkel <hfinkel at anl.gov> wrote:

>
> 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> 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.
>

If widening is expected in the result, the end state IR can still be
checked without the need for end-end testing.


> This is why our tendency to rely only on pass-specific regression tests,
> and not having end-to-end tests, is not necessarily optimal.
>
>
End-end tests still exist, especially performance regression tests --
missing optimizations will likely result in  performance regressions in
real world applications, or else it probably does not matter :)

David



>  -Hal
>
>
> 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.
>
>  -Hal
>
>
> thanks,
>
> David
>
>
>
>
>> 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.
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D36562
>>
>>
>>
>>
>
>
> _______________________________________________
> llvm-commits mailing listllvm-commits at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
>
> _______________________________________________
> llvm-commits mailing listllvm-commits at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170809/d15294d1/attachment.html>