[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

rsmith added a comment. In D67399#1669920 , @rjmccall wrote: > In D67399#1669568 , @jfb wrote: > > > In D67399#1669038 , @dnsampaio > > wrote: > > > > > Indeed our main

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

rjmccall added a comment. In D67399#1669568 , @jfb wrote: > In D67399#1669038 , @dnsampaio wrote: > > > Indeed our main concern is regarding the access widths of loads. As > > mentioned by @rjmccall, most volatile

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

jfb added a comment. In D67399#1669038 , @dnsampaio wrote: > Indeed our main concern is regarding the access widths of loads. As mentioned > by @rjmccall, most volatile bitfields are used to perform memory mapped I/O, > and some hardware only support

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

dnsampaio added a comment. Indeed our main concern is regarding the access widths of loads. As mentioned by @rjmccall, most volatile bitfields are used to perform memory mapped I/O, and some hardware only support them with a specific access width. The spurious load I am more than glad to leave

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

rjmccall added a comment. I have to say that I disagree. The ABI certainly doesn't get to control the language and define what constitutes a volatile access, but when the language has decided that a volatile access is actually performed, I think ABIs absolutely ought to define how they should

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

rsmith added a comment. In D67399#1668759 , @rjmccall wrote: > The exact sequence of volatile accesses is observable behavior, and it's the > ABI's right to define correct behavior for compliant implementations, so we > do need to do this. I'm not

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

rjmccall added a comment. The exact sequence of volatile accesses is observable behavior, and it's the ABI's right to define correct behavior for compliant implementations, so we do need to do this. Diogo, IRGen breaks up bit-field storage units in CGRecordLowering::accumulateBitFields. I'm

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

dnsampaio marked 4 inline comments as done. dnsampaio added a comment. Hi @jfb. In a example such as: struct { int a : 1; int b : 16; } S; extern int expensive_computaion(int v); void foo(volatile S* s){ s->b = expensive_computation(s->b); } There is no guarantee that `s->a` is not

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

jfb added inline comments. Comment at: clang/test/CodeGen/aapcs-bitfield.c:541 // BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], %struct.st9* [[M:%.*]], i32 0, i32 0 +// BE-NEXT:[[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4 // BE-NEXT:

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

lebedev.ri added a comment. In D67399#1664785 , @dnsampaio wrote: > @ostannard might prove me wrong, but according to the AACPS: > > When a volatile bit-field is written, and its container does not overlap > with any non-bit-field member, its >

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

dnsampaio added a comment. @ostannard might prove me wrong, but according to the AACPS: When a volatile bit-field is written, and its container does not overlap with any non-bit-field member, its container must be read exactly once and written exactly once using the access width

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

lebedev.ri resigned from this revision. lebedev.ri added a reviewer: jfb. lebedev.ri added a comment. Herald added a subscriber: dexonsmith. I'm not sure why i'm added as a reviewer here. That being said i don't see why that load is needed there. As i read it, the document only states that if

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

dnsampaio added a comment. This patch could hack clang to generate an extra load. However, my knowledge in the clang code base is not extensive. How could we ensure that the width of loads and stores are the size of the container, and that they don't overlap non-bitfields? Repository: rG

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

dnsampaio created this revision. dnsampaio added reviewers: lebedev.ri, ostannard. Herald added subscribers: cfe-commits, jfb, kristof.beyls. Herald added a project: clang. Bug 43264 This is a first draft to understand what has to be done to fix volatale bitfield access, as to conform to the