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

Sat Sep 21 10:24:42 PDT 2019

rsmith added a comment.

In D67399#1669920 <https://reviews.llvm.org/D67399#1669920>, @rjmccall wrote:

> In D67399#1669568 <https://reviews.llvm.org/D67399#1669568>, @jfb wrote:
>
> > In D67399#1669038 <https://reviews.llvm.org/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 them with a specific access width.
> > >  The spurious load I am more than glad to leave it disable behind a command flag, so it will only appear if the user requests it. See that volatile accesses might have side effects, and for example, an I/O read counter holding an odd number could define that the data is still being processed.
> >
> >
> > Are the cases being addressed in the PR actually relevant to real MMIO, or is this patch following the letter of AAPCS which doesn't actually matter?
>
>
> Again, I think AAPCS is well within its rights to say that certain volatile accesses should be performed with loads and stores of certain widths.  If low-level programmers cannot use bit-fields today with memory-mapped I/O because they cannot trust compilers to produce reasonable accesses, that is a legitimate concern for ABI authors and a legitimate bug for compiler maintainers.

I have no objection to the direction in this patch. I agree that it's important to have a specification that covers this, and while I still think that this has nothing to do with ABI as I believe the term is normally understood, treating it as platform-dependent and specifying it in the same place as the platform ABI might be the most reasonable option. Clearly the AAPCS is more than a Procedure Call Standard, which is fine.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67399/new/

https://reviews.llvm.org/D67399