[LLVM] Add libatomic load/store functions to TargetLibraryInfo

[guiand]

Summary

This allows treating these functions like libcalls.

This patch is a prerequisite to instrumenting them in MSAN:

https://reviews.llvm.org/D83337

[rovka]

Don't you also have to set as Available/Unavailable when initializing the TLI?

llvm/lib/Analysis/TargetLibraryInfo.cpp

1232

Nit: The comment should go above the case statement.

[guiand]

In D83361#2138409, @rovka wrote:

Don't you also have to set as Available/Unavailable when initializing the TLI?

Yes, thanks for the catch! It looks like it's set available by default, there may be platforms that don't have these libatomic functions available.

From a look at RuntimeLibCalls, where these functions are also used (for emitting), it looks like the functions are only explicitly made unavailable on webassembly. (Which is pretty surprising! Could I be missing something here?)

[guiand]

I had gotten the impression that these calls aren't supported on WebAssembly by lines in WebAssemblyRuntimeLibcallSignatures.cpp like:

Table[RTLIB::ATOMIC_LOAD] = unsupported;

But it turns out clang has no problem generating calls to __atomic_load anyway: https://gcc.godbolt.org/z/kMDLJb

And it looks like LLVM looks to provide general support for these functions: https://llvm.org/docs/Atomics.html#libcalls-atomic

[rovka]

Hi again,

I'm sorry, but I don't know much about WebAssembly.

I would suggest taking the opposite approach: mark as unavailable in general, and as available only on platforms where you know for sure they exist - based on the docs you linked, that's probably anything that has C++11 support.

Maybe someone else with more experience in this area can chime in.

[jfb]

@dschuff can help answer the WebAssembly question.

[guiand]

I've added a short list of architectures known to use atomic operations and observed to generate calls to __atomic_load/store

[rovka]

Thanks for the update. This looks fine to me as is, but I'll defer the final LGTM to someone that knows this area a bit better.

llvm/lib/Analysis/TargetLibraryInfo.cpp

175

Nit: this doesn't need to move.

[guiand]

llvm/lib/Analysis/TargetLibraryInfo.cpp

175

I moved it just up one scope so I can use it in the if statement below.

[efriedma]

llvm/lib/Analysis/TargetLibraryInfo.cpp

555

Checking for specific targets seems like a trap that's going to "randomly" break more obscure targets.

I think I'd be happy to say these are "always" available, in the sense that we should assume a function named __atomic_load is always the libatomic one. (In practice, whether they're actually available at link-time might vary depending on how the user links their code, but we shouldn't be introducing new calls to these anyway.)

[guiand]

@efriedma that makes sense. I've reverted to the previous patch where these functions are always available.

[efriedma]

LGTM

llvm/include/llvm/Analysis/TargetLibraryInfo.def

 TLI_DEFINE_ENUM_INTERNAL(atanh_finite)
 TLI_DEFINE_STRING_INTERNAL("__atanh_finite")
 /// float __atanhf_finite(float x);
 TLI_DEFINE_ENUM_INTERNAL(atanhf_finite)
 TLI_DEFINE_STRING_INTERNAL("__atanhf_finite")
 /// long double __atanhl_finite(long double x);
 TLI_DEFINE_ENUM_INTERNAL(atanhl_finite)
 TLI_DEFINE_STRING_INTERNAL("__atanhl_finite")
+/// void __atomic_load(size_t size, void *mptr, void *vptr, int smodel);
+TLI_DEFINE_ENUM_INTERNAL(atomic_load)
+TLI_DEFINE_STRING_INTERNAL("__atomic_load")
+/// void __atomic_store(size_t size, void *mptr, void *vptr, int smodel);
+TLI_DEFINE_ENUM_INTERNAL(atomic_store)
+TLI_DEFINE_STRING_INTERNAL("__atomic_store")
 /// double __cosh_finite(double x);
 TLI_DEFINE_ENUM_INTERNAL(cosh_finite)
 TLI_DEFINE_STRING_INTERNAL("__cosh_finite")
 /// float __coshf_finite(float x);
 TLI_DEFINE_ENUM_INTERNAL(coshf_finite)
 TLI_DEFINE_STRING_INTERNAL("__coshf_finite")
 /// long double __coshl_finite(long double x);
 TLI_DEFINE_ENUM_INTERNAL(coshl_finite)

llvm/lib/Analysis/TargetLibraryInfo.cpp

static void initialize(TargetLibraryInfoImpl &TLI, const Triple &T,
 
   // __small_printf and friends are only available on Emscripten.
   if (T.getOS() != Triple::Emscripten) {
     TLI.setUnavailable(LibFunc_small_printf);
     TLI.setUnavailable(LibFunc_small_sprintf);
     TLI.setUnavailable(LibFunc_small_fprintf);
   }
 
   if (T.isOSWindows() && !T.isOSCygMing()) {
     // XXX: The earliest documentation available at the moment is for VS2015/VC19:
     // https://docs.microsoft.com/en-us/cpp/c-runtime-library/floating-point-support?view=vs-2015
     // XXX: In order to use an MSVCRT older than VC19,
     // the specific library version must be explicit in the target triple,
     // e.g., x86_64-pc-windows-msvc18.
     bool hasPartialC99 = true;
     if (T.isKnownWindowsMSVCEnvironment()) {
       unsigned Major, Minor, Micro;
if (T.isNVPTX()) {
     TLI.setAvailable(LibFunc_nvvm_reflect);
   } else {
     TLI.setUnavailable(LibFunc_nvvm_reflect);
   }
 
   TLI.addVectorizableFunctionsFromVecLib(ClVectorLibrary);
 }
 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl() {
   // Default to everything being available.
   memset(AvailableArray, -1, sizeof(AvailableArray));
 
   initialize(*this, Triple(), StandardNames);
 }
 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl(const Triple &T) {
   // Default to everything being available.
bool TargetLibraryInfoImpl::isValidProtoForLibFunc(const FunctionType &FTy,
   // void operator delete(void*, unsigned long, align_val_t)
   case LibFunc_ZdlPvmSt11align_val_t:
   // void operator delete[](void*, unsigned int, align_val_t);
   case LibFunc_ZdaPvjSt11align_val_t:
   // void operator delete[](void*, unsigned long, align_val_t);
   case LibFunc_ZdaPvmSt11align_val_t:
     return (NumParams == 3 && FTy.getParamType(0)->isPointerTy());
 
+  // void __atomic_load(size_t, void *, void *, int)
+  case LibFunc_atomic_load:
+  // void __atomic_store(size_t, void *, void *, int)
+  case LibFunc_atomic_store:
+    return (NumParams == 4 && FTy.getParamType(0)->isIntegerTy() &&
+            FTy.getParamType(1)->isPointerTy() &&
+            FTy.getParamType(2)->isPointerTy() &&
+            FTy.getParamType(3)->isIntegerTy());
+
   case LibFunc_memset_pattern16:
     return (!FTy.isVarArg() && NumParams == 3 &&
             FTy.getParamType(0)->isPointerTy() &&
             FTy.getParamType(1)->isPointerTy() &&
             FTy.getParamType(2)->isIntegerTy());
 
   case LibFunc_cxa_guard_abort:
   case LibFunc_cxa_guard_acquire:

llvm/unittests/Analysis/TargetLibraryInfoTest.cpp

parseAssembly(
       "declare i64 @__strlcpy_chk(i8*, i8*, i64, i64)\n"
       "declare i32 @__vsnprintf_chk(i8*, i64, i32, i64, i8*, %struct*)\n"
       "declare i32 @__vsprintf_chk(i8*, i32, i64, i8*, %struct*)\n"
 
       "declare i8* @memalign(i64, i64)\n"
       "declare i8* @mempcpy(i8*, i8*, i64)\n"
       "declare i8* @memrchr(i8*, i32, i64)\n"
 
+      "declare void @__atomic_load(i64, i8*, i8*, i32)\n"
+      "declare void @__atomic_store(i64, i8*, i8*, i32)\n"
+
       // These are similar to the FILE* fgetc/fputc.
       "declare i32 @_IO_getc(%struct*)\n"
       "declare i32 @_IO_putc(i32, %struct*)\n"
 
       "declare i32 @__isoc99_scanf(i8*, ...)\n"
       "declare i32 @__isoc99_sscanf(i8*, i8*, ...)\n"
       "declare i8* @__strdup(i8*)\n"
       "declare i8* @__strndup(i8*, i64)\n"