[libc++] XFAIL align.pass.cpp for PowerPC LE

[amyk]

Summary

This patch XFAILs the align.pass.cpp for PowerPC (LE).

It appears that this test will fail on Power for the LLIArr2 and Padding structs within the test,
as the assert for alignof(AtomicImpl) >= sizeof(AtomicImpl) will be false. In this case, these structs
presumably should not be lock-free, so we currently XFAIL this for now.

The failure was discovered after D97913 was committed. It looks like alignof(AtomicImpl) < sizeof(AtomicImpl),
even prior to this commit, but this test began running on Power after D97913, whereas we were
not running align.pass.cpp before.

This patch addresses https://bugs.llvm.org/show_bug.cgi?id=51548 by temporarily XFAILing the test
in order to investigate it further.

[ldionne]

Can you explain why those assertion fails? Am I wrong to think this is a compiler bug on those targets?

libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp

21

Also, where do we define the powerpc64le and powerpcle Lit features? IMO, this should be something of the form XFAIL: target=<the-triple-you-target>.

[Mordante]

This seems like it addresses https://bugs.llvm.org/show_bug.cgi?id=51548. Can you please add that bug number in the description?

[amyk]

Thanks @Mordante! I've added the Bugzilla to the description.

Thanks @ldionne for taking a look at the patch. We are still looking into this and decided to XFAIL the test temporarily. I see that the assertion occurs because the following structs:

struct LLIArr2 { long long int i[2]; }
struct Padding { char c; /* padding */ long long int i; }

do not have the alignment that is expected (alignof() returns 8, while sizeof() returns 16), hence the assertion fails. Since this assertion fails, presumably the this->is_lock_free() check should not have been true to begin with.

---

@lkail assisted me in looking a bit more into this since I've posted the patch. For example, if I have a test of just the two structs:

#include <atomic>
#include <cassert>
#include <stdio.h>

template <typename T>
struct atomic_test : public std::__atomic_base<T> {
  atomic_test() {
    if (this->is_lock_free()) {
      using AtomicImpl = decltype(this->__a_);
      printf("alignof() = %lu, sizeof() = %lu\n",
           alignof(AtomicImpl), sizeof(AtomicImpl));
      assert(alignof(AtomicImpl) >= sizeof(AtomicImpl) &&
             "expected natural alignment for lock-free type");
    }
  }
};

int main(int, char**) {

#define CHECK_ALIGNMENT(T)                                                     \
  do {                                                                         \
    typedef T type;                                                            \
    atomic_test<type> t;                                                       \
  } while (0)

  CHECK_ALIGNMENT(struct LLIArr2 { long long int i[2]; });
  CHECK_ALIGNMENT(struct Padding { char c; /* padding */ long long int i; });

  return 0;
}

This may have something to do with the test using the PowerPC GNU libatomic. The executable that is compiled looks like it is linking in the system libatomic by default, and doing so causes the assert.

$ clang++ reduced.cpp -O3 -stdlib=libc++ -L/home/amyk/llvm/build/lib -latomic -Wl,-rpath=/home/amyk/llvm/build/lib

$ ./a.out
alignof() = 8, sizeof() = 16
a.out: foo.cc:13: atomic_test<LLIArr2>::atomic_test() [T = LLIArr2]: Assertion `alignof(AtomicImpl) >= sizeof(AtomicImpl) && "expected natural alignment for lock-free type"' failed.
Aborted

$ ldd a.out
	linux-vdso64.so.1 (0x00007817de440000)
	libatomic.so.1 => /usr/lib/powerpc64le-linux-gnu/libatomic.so.1 (0x00007817de400000)
	libc++.so.1 => /home/amyk/llvm/build/lib/libc++.so.1 (0x00007817de2f0000)
	libc++abi.so.1 => /home/amyk/llvm/build/lib/libc++abi.so.1 (0x00007817de280000)
	libm.so.6 => /lib/powerpc64le-linux-gnu/libm.so.6 (0x00007817de130000)
	libgcc_s.so.1 => /lib/powerpc64le-linux-gnu/libgcc_s.so.1 (0x00007817de0f0000)
	libc.so.6 => /lib/powerpc64le-linux-gnu/libc.so.6 (0x00007817ddec0000)
	libpthread.so.0 => /lib/powerpc64le-linux-gnu/libpthread.so.0 (0x00007817dde70000)
	librt.so.1 => /lib/powerpc64le-linux-gnu/librt.so.1 (0x00007817dde40000)
	/lib64/ld64.so.2 (0x00007817de460000)

However, when we build and use the libatomic from compiler-rt, and compile the test, this test passes.

Thus, because it looks like a problem in GNU libatomic vs. compiler-rt and since the problem was exposed recently, this is why we thought it may be appropriate to XFAIL it until we find a solution.

I am not sure if using the system libatomic is expected and if not, I am not sure if making it use compiler-rt's libatomic is something that can be achieve through the libc++ test CMake files (or something similar)?

libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp

21

Thanks for the suggestion.
I've put those LIT features there as they were previously used before in other tests, but I can try the approach with XFAIL: target=.

[amyk]

Update the target to XFAIL and add a brief description that the test fails on Power.

[ldionne]

I'm going to approve this if it solves your problem in the short term. However, I would strongly advise figuring out what the underlying issue is on this platform. It looks like we should be using compiler-rt instead of libatomic, and yeah, that should be achievable from the CMake.

libcxx/test/libcxx/atomics/atomics.align/align.pass.cpp

 // ABI change for vector variants since the constructor code for that is
 // different if one were to compile with architecture-specific vector
 // extensions enabled.
 // This however isn't ABI breaking as it was impossible for any code to trigger
 // this without using libc++ internals.
 
 // GCC currently fails because it needs -fabi-version=6 to fix mangling of
 // std::atomic when used with __attribute__((vector(X))).
 // XFAIL: gcc
 
+// This fails on PowerPC, as the LLIArr2 and Padding structs do not have
+// adequate alignment, despite these types returning true for the query of
+// being lock-free. This is an issue that occurs when linking in the
+// PowerPC GNU libatomic library into the test.
+// XFAIL: target=powerpc{{.*}}le-unknown-linux-gnu
+
 // <atomic>
 
 // Verify that the content of atomic<T> is properly aligned if the type is
 // lock-free. This can't be observed through the atomic<T> API. It is
 // nonetheless required for correctness of the implementation: lock-free implies
 // that ISA instructions are used, and these instructions assume "suitable
 // alignment". Supported architectures all require natural alignment for
 // lock-freedom (e.g. load-linked / store-conditional, or cmpxchg).