[PATCH] D145214: [TSAN] add support for riscv64

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.


================
Comment at: compiler-rt/CMakeLists.txt:529
 # natively, but supports --as-needed/--no-as-needed for GNU ld compatibility.
-if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc")
+if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc|riscv")
   list(APPEND SANITIZER_COMMON_LINK_LIBS -Wl,--as-needed atomic -Wl,--no-as-needed)
----------------
This is only to work around a long-standing bug in GCC's atomics implementation for RISC-V. If you build compiler-rt with Clang you don't need this, and in theory future versions of GCC (possibly trunk already fixes this, I'm not sure of the status of it).


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cpp:60
 template<typename T> T func_xchg(volatile T *v, T op) {
-  T res = __sync_lock_test_and_set(v, op);
+  T res = __atomic_test_and_set(v, op);
   // __sync_lock_test_and_set does not contain full barrier.
----------------
?


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_platform.h:971
     };
-    const uptr indicator = 0x0e0000000000ull;
+    const uptr indicator = 0x0f0000000000ull;
     const uptr ind_lsb = 1ull << LeastSignificantSetBitIndex(indicator);
----------------
?


================
Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:270-271
+#elif SANITIZER_RISCV64
+  // the bottom half of vma is allocated for userspace
+  vmaSize = vmaSize + 1;
+# if !SANITIZER_GO
----------------
Why don't other architectures need the +1?


================
Comment at: compiler-rt/lib/tsan/tests/CMakeLists.txt:59
+  if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "riscv")
+    list(APPEND TSAN_UNITTEST_LINK_FLAGS -latomic)
+  endif()
----------------
Same as before


================
Comment at: compiler-rt/test/tsan/test.h:77
+#elif defined(__riscv) && __riscv_xlen == 64
+const int kPCInc = 2;
 #else
----------------
What's this the length of? RVC is optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145214