Re: [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

2017-09-04 Thread Xinliang David Li via cfe-commits
On Mon, Sep 4, 2017 at 1:57 AM, Chandler Carruth wrote: > On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits < > llvm-comm...@lists.llvm.org> wrote: > >> Nevertheless, I think that you've convinced me that this is a least-bad >> solution. I'll want a flag preserving

Re: [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

2017-09-04 Thread Xinliang David Li via cfe-commits
On Mon, Sep 4, 2017 at 9:17 AM, Hal Finkel wrote: > > On 09/04/2017 03:57 AM, Chandler Carruth wrote: > > On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits < > llvm-comm...@lists.llvm.org> wrote: > >> Nevertheless, I think that you've convinced me that this is a

Re: [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

2017-09-04 Thread Hal Finkel via cfe-commits
On 09/04/2017 03:57 AM, Chandler Carruth wrote: On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits > wrote: Nevertheless, I think that you've convinced me that this is a least-bad solution. I'll want a flag

Re: [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

2017-09-04 Thread Chandler Carruth via cfe-commits
On Sun, Sep 3, 2017 at 10:41 PM Hal Finkel via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > Nevertheless, I think that you've convinced me that this is a least-bad > solution. I'll want a flag preserving the old behavior. Something like > -fwiden-bitfield-accesses (modulo bikeshedding). >

Re: [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

2017-09-03 Thread Hal Finkel via cfe-commits
On 09/03/2017 11:22 PM, Wei Mi wrote: On Sun, Sep 3, 2017 at 8:55 PM, Hal Finkel wrote: On 09/03/2017 10:38 PM, Xinliang David Li wrote: Store forwarding stall cost is usually much higher compared with a load hitting L1 cache. For instance, on Haswell, the latter is ~4

Re: [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

2017-09-03 Thread Hal Finkel via cfe-commits
On 09/04/2017 12:12 AM, Xinliang David Li wrote: On Sun, Sep 3, 2017 at 9:23 PM, Hal Finkel > wrote: On 09/03/2017 11:06 PM, Xinliang David Li wrote: I think we can think this in another way. For modern CPU architectures which

Re: [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

2017-09-03 Thread Xinliang David Li via cfe-commits
On Sun, Sep 3, 2017 at 9:23 PM, Hal Finkel wrote: > > On 09/03/2017 11:06 PM, Xinliang David Li wrote: > > I think we can think this in another way. > > For modern CPU architectures which supports store forwarding with store > queues, it is generally not "safe" to blindly do

Re: [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

2017-09-03 Thread Hal Finkel via cfe-commits
On 09/03/2017 11:06 PM, Xinliang David Li wrote: I think we can think this in another way. For modern CPU architectures which supports store forwarding with store queues, it is generally not "safe" to blindly do local optimizations to widen the load/stores Why not widen stores? Generally

Re: [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

2017-09-03 Thread Wei Mi via cfe-commits
On Sun, Sep 3, 2017 at 8:55 PM, Hal Finkel wrote: > > On 09/03/2017 10:38 PM, Xinliang David Li wrote: > > Store forwarding stall cost is usually much higher compared with a load > hitting L1 cache. For instance, on Haswell, the latter is ~4 cycles, while > the store forwarding

Re: [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

2017-09-03 Thread Xinliang David Li via cfe-commits
I think we can think this in another way. For modern CPU architectures which supports store forwarding with store queues, it is generally not "safe" to blindly do local optimizations to widen the load/stores without sophisticated inter-procedural analysis. Doing so will run the risk of greatly

Re: [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

2017-09-03 Thread Hal Finkel via cfe-commits
On 09/03/2017 10:38 PM, Xinliang David Li wrote: Store forwarding stall cost is usually much higher compared with a load hitting L1 cache. For instance, on Haswell, the latter is ~4 cycles, while the store forwarding stalls cost about 10 cycles more than a successful store forwarding, which

Re: [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

2017-09-03 Thread Xinliang David Li via cfe-commits
Store forwarding stall cost is usually much higher compared with a load hitting L1 cache. For instance, on Haswell, the latter is ~4 cycles, while the store forwarding stalls cost about 10 cycles more than a successful store forwarding, which is roughly 15 cycles. In some scenarios, the store

Re: [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

2017-09-03 Thread Hal Finkel via cfe-commits
On 09/03/2017 03:44 PM, Wei Mi wrote: On Sat, Sep 2, 2017 at 6:04 PM, Hal Finkel wrote: On 08/22/2017 10:56 PM, Wei Mi via llvm-commits wrote: On Tue, Aug 22, 2017 at 7:03 PM, Xinliang David Li wrote: On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth

Re: [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

2017-09-03 Thread Wei Mi via cfe-commits
On Sat, Sep 2, 2017 at 6:04 PM, Hal Finkel wrote: > > On 08/22/2017 10:56 PM, Wei Mi via llvm-commits wrote: >> >> On Tue, Aug 22, 2017 at 7:03 PM, Xinliang David Li >> wrote: >>> >>> >>> On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator >>>

Re: [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

2017-09-02 Thread Hal Finkel via cfe-commits
On 08/22/2017 10:56 PM, Wei Mi via llvm-commits wrote: On Tue, Aug 22, 2017 at 7:03 PM, Xinliang David Li wrote: On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator wrote: chandlerc added a comment. I'm really not a fan of the

Re: [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

2017-08-22 Thread Wei Mi via cfe-commits
On Tue, Aug 22, 2017 at 7:03 PM, Xinliang David Li wrote: > > > On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator > wrote: >> >> chandlerc added a comment. >> >> I'm really not a fan of the degree of complexity and subtlety that this

Re: [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

2017-08-22 Thread Xinliang David Li via cfe-commits
On Tue, Aug 22, 2017 at 7:46 PM, Chandler Carruth wrote: > > > On Tue, Aug 22, 2017 at 7:18 PM Xinliang David Li via llvm-commits < > llvm-comm...@lists.llvm.org> wrote: > >> On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits < >> llvm-comm...@lists.llvm.org>

Re: [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

2017-08-22 Thread Chandler Carruth via cfe-commits
On Tue, Aug 22, 2017 at 7:18 PM Xinliang David Li via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits < > llvm-comm...@lists.llvm.org> wrote: > >> On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits < >>

Re: [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

2017-08-22 Thread Hal Finkel via cfe-commits
On 08/22/2017 09:18 PM, Xinliang David Li via llvm-commits wrote: On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits > wrote: On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits

Re: [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

2017-08-22 Thread Xinliang David Li via cfe-commits
On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator < >>

Re: [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

2017-08-22 Thread Chandler Carruth via cfe-commits
On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator < > revi...@reviews.llvm.org> wrote: > >> chandlerc added a comment. >> >> I'm really not a fan of the degree of complexity

Re: [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

2017-08-22 Thread Xinliang David Li via cfe-commits
On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator < revi...@reviews.llvm.org> wrote: > chandlerc added a comment. > > I'm really not a fan of the degree of complexity and subtlety that this > introduces into the frontend, all to allow particular backend optimizations. > > I feel