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

[John McCall via Phabricator via cfe-commits]

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 be compiled.  Volatile accesses are quite unusual in that they are used for a purpose — memory-mapped I/O — that is extremely dependent for correctness on the exact instructions used to implement it.  IIRC there are architectures that for whatever reason provide two different load instructions where only one of those instructions actually performs memory-mapped I/O correctly.  Certainly the exact width of load or store is critical for correctness.  So while I have certainly seen ABI overreach before and pushed back on it, for this one case I think ABI involvement is totally appropriate, and in fact I wish more ABIs specified how they expect volatile accesses to be performed.

It is, of course, somewhat unfortunate that a corner-case use case like memory-mapped I/O has been privileged with such a core position in the language, but that's how it is.

I do think that AAPCS's specific request that the compiler emit spurious loads when storing to volatile bit-fields is a bad idea, and I think it would be fine to push back on that part, and perhaps also on the idea that compound assignments and increments should load twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67399