Clarify Atomic type alignment requirements

[lenary]

This patch documents alignment requirements for atomic types. This issue came up in https://reviews.llvm.org/D57450

Using a small test program, I have deduced the rules for atomic type size and alignment in GCC. There are more details about how I did this in a comment on that review, which I quote below (MaxAtomicPromoteWidth is a target-specific configuration option in Clang, which controls changing the alignment of atomic types based on their size):

I've been looking at how GCC and Clang differ in how they deal with issues around size and alignment of atomic objects.

All types of size less than or equal to MaxAtomicPromoteWidth, when they are turned into their atomic variant, will: a) have their size rounded up to a power of two, and b) have their alignment changed to that same power of two.

The following Gist contains:

• a C program which will print the size and alignment of a variety of strangely sized and aligned objects, and their atomic variants.
• the output of this program when run through gcc and clang, specifying rv(32,64)ima(,fd) and (ilp32,lp64)(,f) (matching ABIs to architectures). I used riscv64-unknown-linux-gnu-gcc for the avoidance of doubt (this compiler allows you to compile for rv32 by specifying a 32-bit -march value).
• the diffs in those outputs for an single ABI, between the two different compilers.
  The gist is here: https://gist.github.com/lenary/2e977a8af33876ba8e0605e98c4e1b0d

There are some differences between GCC and Clang, that seem not to be risc-v specific (I saw them when running the C program on my mac)

• Clang ensures zero-sized objects become char-sized when they become atomic. GCC leaves them zero-sized. This is documented in ASTContext.cpp line 2130, to ensure the atomic operation is generated.
• GCC seems not to round up the size and alignment of non-power-of-two-sized structures.

I think this patch needs to be updated to ensure there are no differences in objects of power-of-two size. To do this, both riscv64 and riscv32, should have a MaxAtomicPromoteWidth of 128.

[lenary]

ping?

[lenary]

I am not happy with the exception around non-power-of-two-sized atomic types. Given this ABI is relatively new, I have updated the proposed addition so non-power-of-two sized atomic types are padded to the next power-of-two, and then aligned to the same power-of-two.

The System V ABI does not define this case. This behaviour allegedly matches the Darwin ABI, not that I can find any documentation on that beyond a comment in the LLVM bug tracker.

It seems reasonable to settle on defining this behaviour to avoid incompatibility between code involving atomics, when compiled with different toolchains.

[aswaterman]

x86-64 clang, x86-64 gcc, and RV64 GCC all print "4" when running this program:

#include <stdio.h>

struct s { int a; int b; int c; };
int main() { printf("%ld\n", alignof(std::atomic<s>)); return 0; }

I'm reluctant to (a) change the ABI for GCC and (b) diverge from what x86-64 GCC and clang are doing here.

[palmer-dabbelt]

GCC's alignment rules are complicated to write down, but they seem to do reasonable things for my test cases:

• struct { int[2]; } aligns to 8, which makes as it lets you do this more efficiently with LR/SC (we don't have byte or half-word atomics).
• struct { char[3]; } aligns to 1. This isn't how I'd do it, but it matches x86 and arm64 on clang and GCC so it seems like the best option.
• struct { char[16]; } aligns to 16, which makes sense as we'll eventually need a way to handle dual-word CAS and that might end up providing an efficient implementation for this as well.

Given this is the ABI that already exists, I don't see a compelling reason to change it.

[jim-wilson]

We fixed the ABI when we upstreamed glibc. Changing it now would be problematic, as this might break existing linux distros. So we should only be documenting the existing (gcc implemented) ABI, not changing it, unless we find a serious problem.

We do need compatibility between llvm and gcc though. So I looked at the details of what gcc is doing. The key detail is in gcc/tree.c which does

      if (((type_quals & TYPE_QUAL_ATOMIC) == TYPE_QUAL_ATOMIC))
        {
          /* See if this object can map to a basic atomic type.  */
          tree atomic_type = find_atomic_core_type (type);
          if (atomic_type)
            {
              /* Ensure the alignment of this type is compatible with           
                 the required alignment of the atomic type.  */
              if (TYPE_ALIGN (atomic_type) > TYPE_ALIGN (t))
                SET_TYPE_ALIGN (t, TYPE_ALIGN (atomic_type));
            }
        }

The tree_atomic_core_type function only looks at the type size, and returns the corresponding integer atomic type if there is one of the same size. So if you declare a struct as atomic, and it happens to have the same size as a supported atomic integer type, then it gets the alignment of that integer type. Otherwise, it gets the normal struct alignment, which is the max alignment of the fields.

This code is part of the original patch that added the atomic qualifier support.
https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00493.html
Checking the C standard, it does say that the size and alignment of an atomic qualified type need not be the same as the equivalent unqualified type. The gcc choice was probably made for efficiency reasons. If we give a struct the same alignment as the same size integer type, then we can use the normal atomic load/store instructions to access it.

If clang is implementing atomic structure types differently than gcc, then this isn't a RISC-V problem, it is a generic clang/gcc compatibility issue which needs to be handled elsewhere. Just checking x86 with your atomic-alignment.c testcase, I see that there are x86 gcc/clang differences. It looks like clang has an additional rule, if an atomic struct is not power of 2 sized, then it is padded to the next power of 2. So a struct of size 3 made atomic is given size and alignment of 4 in clang but size 3 align 1 in gcc. This definitely needs to be handled at a higher level. I can start a thread on the gcc mailing lists.

[lenary]

My understanding of std::atomic is that the alignment it will give you depends both on which version of the c++ library implementation and on which compiler is used, hence pointing to _Atomic, which only depends on your compiler. There are more details in this thread on the LLVM bug tracker: https://bugs.llvm.org/show_bug.cgi?id=26462

The clang/llvm approach to this seems to be that it should be well defined by the ABI, and they are have so far been unwilling to change until ABI specifications are updated. Unfortunately it also seems that while the darwin ABI has thought about non-power-of-two-sized atomic types, they have failed to write anything down so far (in documents I could find, at least), and have ignored the fact that gcc may or may not be producing code for their system that matches the ABI.

Thanks for taking this up on the GCC mailing list, I look forward to seeing the outcome of that discussion, and hope we can find a good compromise.

[jim-wilson]

Thanks for the link to the llvm bug. The gcc thread is at
https://gcc.gnu.org/ml/gcc/2019-08/msg00191.html
and has a link to an x86 ABI discussion which started in Dec 2018 and never got resolved. Probably we need to push on the x86 ABI discussion because everyone cares about the x86 ABI, and then propagate that decision to the other architectures, with an optional opt out for people who want the other ABI.

It looks like the current LLVM solution is better than the current GCC solution, as the LLVM one requires less locking. So I think it is more likely that gcc will change than llvm, but this is going to be a mess. This will probably require a libstdc++ ABI version increment for instance. I'd rather leave the general gcc stuff to other people, and only worry about the RISC-V specific stuff, so this is easier if the x86 ABI is fixed first, but that could be a while.