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

Thu Sep 12 18:10:44 PDT 2019

rsmith added a comment.

In D67399#1668759 <https://reviews.llvm.org/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 convinced; I think the AAPCS is overstepping its bounds here by trying to specify this. Whether you get a volatile load for a full-width bitfield access doesn't seem to have anything to do with ABI; implementations that differ on this detail will be able to produce code that links together and works together just fine. I think this is instead a question of what guarantees the implementation wants to give about how it treats volatile operations and how it lowers them to hardware. That should be specified, sure (and in fact C++ at least requires us to document what we do), but it's not ABI any more than (for example) the choice of whether a volatile access to a local variable must actually issue load / store instructions -- or which instructions must be produced -- is ABI. I have no particular opinion on whether we should make this change -- volatile bit-fields are pretty weird and broken regardless. But I don't think we should view it as being part of the ABI, and just as with the question of whether plain bit-fields are signed, we should tell the psABI folks that the question of what constitutes a volatile access and the semantics of such an access is not ABI and not up to them.

(Similar example: on Windows x86 targets, `volatile` accesses are atomic by default (controlled by compiler switch). `clang-cl` follows `cl` in this regard, but we don't consider it to be part of the ABI, and in the regular clang driver, we do not treat `volatile` accesses as atomic when targeting Windows x86.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67399