[PATCH] D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V

[Gokturk Yuksek via Phabricator via llvm-commits]

gokturk added a comment.

In D68964#1708804 <https://reviews.llvm.org/D68964#1708804>, @smeenai wrote:

> libc++ also has a version of this check (https://github.com/llvm/llvm-project/blob/master/libcxx/cmake/Modules/CheckLibcxxAtomic.cmake). Does that need to be adjusted as well?


It does create false positives. If I use the following code sample from the libcxx cmake test:

  #include <cstdint>
  #include <atomic>
  std::atomic<uintptr_t> x;
  std::atomic<uintmax_t> y;
  int main(int, char**) {
    return x + y;
  }

g++ compiles it just fine, with or without the 'A' extension defined (-march=rv64imafd vs -march=rv64imfd). It seems like g++ is inlining the following two functions in the binary:

  std::__atomic_base<unsigned long>::operator unsigned long() const;
  std::operator&(std::memory_order, std::__memory_order_modifier;

but performing the `x + y` as two sign-extended integer addition:

  ...
  # initialize and sign-extend x, store it in register s1
  jal	ra,3a8 <std::__atomic_base<unsigned long>::operator unsigned long() const>
  mv	a5,a0
  sext.w	s1,a5
  ...
  # Initialize and sign-extend y, store it in register a5
  jal	ra,3a8 <std::__atomic_base<unsigned long>::operator unsigned long() const>
  mv	a5,a0
  sext.w	a5,a5
  # Perform x + y, sign extend the result twice (??)
  addw	a5,s1,a5
  sext.w	a5,a5
  sext.w	a5,a5

As a result, no need for libatomic in the compilation. If I tweak the code sample a little bit so that `x + y` is a separate statement:

  #include <cstdint>
  #include <atomic>
  std::atomic<uintptr_t> x;
  std::atomic<uintmax_t> y;
  int main(int, char**) {
    x += y;
    return x;
  }

this still compiles fine under the 'A' extension:

  $ g++ -march=rv64imafd -std=c++11 -nostdlib libcxx-atomic-check.cpp 
  /usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: warning: cannot find entry symbol _start; defaulting to 0000000000000298

but fails without the 'A' extension:

  $ g++ -march=rv64imfd -std=c++11 -nostdlib libcxx-atomic-check.cpp 
  /usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: warning: cannot find entry symbol _start; defaulting to 0000000000000310
  /usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld: /tmp/ccNebYb9.o: in function `.L0 ':
  libcxx-atomic-check.cpp:(.text._ZNSt13__atomic_baseImEpLEm[_ZNSt13__atomic_baseImEpLEm]+0x28): undefined reference to `__atomic_fetch_add_8'
  collect2: error: ld returned 1 exit status

I think it needs to be fixed as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68964