[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

Sam Elliott via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 27 04:02:26 PDT 2019


lenary added a comment.

In D57450#1641308 <https://reviews.llvm.org/D57450#1641308>, @jyknight wrote:

> In D57450#1641190 <https://reviews.llvm.org/D57450#1641190>, @lenary wrote:
>
> > @jyknight I hear where you're coming from. I'll see what I can do about the psABI document.
> >
> > In that ticket, it's mentioned that the Darwin ABI explicitly says that non-power-of-two atomic types should be padded and realigned, but I cannot find any documentation explaining this. That would be useful, given presumably GCC does have to pad/align on Darwin.
>
>
> AFAIK, there is no such documentation, at least publicly. Possibly Apple has some internally, but I suspect it more likely just some in-person conversation or something.
>
> GCC is not really supported on Darwin, so I suspect it just gets it wrong.


To keep everyone updated, this has bubbled over into a thread on the GCC Mailing list: https://gcc.gnu.org/ml/gcc/2019-08/msg00191.html - Potentially this is a route to a resolution on non-RISC-V platforms (specifically x86).

> 
> 
>> Then the only outstanding question relates to zero-sized atomics, which GCC does not pad, but I think Clang has to pad to get the semantics correct, based on this comment: https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ASTContext.cpp#L2176
> 
> The semantics in GCC are that you can create such an object, but any attempt to load or store it will result in a compile-time error. E.g., "error: argument 1 of ‘__atomic_load’ must be a pointer to a nonzero size object". So I don't think there's really an issue there.

Neat, ok.

I think the feeling with this ticket and the RISC-V backend is:

- We match GCC for power-of-two-sized atomics
- We don't match for non-power-of-two sized atomics (as on any other platform)

This feels like that should be enough for this patch to be merged, and in a future patch we can address the fall-out of ABIs changing across GCC and clang on more platforms than just RISC-V?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57450





More information about the cfe-commits mailing list