[atomics] New warning -Watomic-libcall when atomic operation expands to a library call

[sdardis]

Summary

Targets typically support atomics that are word sized (e.g. 32 or 64 bit) or
half word sized (e.g. 32 bit on 64 bit systems). For larger sizes, some
targets can perform them directly (x86, ARM), other must expand them into
a library call. This patch adds the optional warning -Watomic-libcall which
warns when a atomic operation cannot be expanded inline and must use a
library call.

[joerg]

While I agree with the general sentiment, the check doesn't work at all. For example, ARMv4 always has to use libcalls. It's not even true that libcalls are slow, i.e. on true non-parallel architectures, the libcall can be as simple as a restartable atomic sequence and is almost cost free beyond the function call overhead.

[joerg]

More precisely, this should re-use the logic from __atomic_is_lock_free and not reinvent it.

[sdardis]

Update comment and test for atomic expansion.

[vkalintiris]

I removed myself from the reviewers because I'm not quite familiar with this area. @joerg I added you as reviewer because you've already commented, feel free to undo my change if you don't have time to review this.

[sdardis]

Ping.

include/clang/Basic/DiagnosticGroups.td

 def BackendOptimizationRemarkMissed : DiagGroup<"pass-missed">;
 def BackendOptimizationRemarkAnalysis : DiagGroup<"pass-analysis">;
 def BackendOptimizationFailure : DiagGroup<"pass-failed">;
 
 // Instrumentation based profiling warnings.
 def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
 def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
 
+def AtomicLibcall : DiagGroup<"atomic-libcall">;
+
 // AddressSanitizer frontent instrumentation remarks.
 def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;
 
 // Issues with serialized diagnostics.
 def SerializedDiagnostics : DiagGroup<"serialized-diagnostics">;
 
 // A warning group for warnings about code that clang accepts when
 // compiling CUDA C/C++ but which is not compatible with the CUDA spec.
 def CudaCompat : DiagGroup<"cuda-compat">;
 
 // A warning group for things that will change semantics in the future.
 def FutureCompat : DiagGroup<"future-compat">;
 
 def InvalidOrNonExistentDirectory : DiagGroup<"invalid-or-nonexistent-directory">;
 
 def OptionIgnored : DiagGroup<"option-ignored">;
 
 def UnknownArgument : DiagGroup<"unknown-argument">;
 
 // A warning group for warnings about code that clang accepts when
 // compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
-def SpirCompat : DiagGroup<"spir-compat">;
+def SpirCompat : DiagGroup<"spir-compat">;
 

include/clang/Basic/DiagnosticSemaKinds.td

 def err_block_return_missing_expr : Error<
   "non-void block should return a value">;
 def err_func_def_incomplete_result : Error<
   "incomplete result type %0 in function definition">;
 def err_atomic_specifier_bad_type : Error<
   "_Atomic cannot be applied to "
   "%select{incomplete |array |function |reference |atomic |qualified |}0type "
   "%1 %select{||||||which is not trivially copyable}0">;
+def warn_atomic_builtin_expands_to_libcall : Warning<
+  "atomic builtin expands to library call">, InGroup<AtomicLibcall>, DefaultIgnore;
 
 // Expressions.
 def ext_sizeof_alignof_function_type : Extension<
   "invalid application of '%select{sizeof|alignof|vec_step}0' to a "
   "function type">, InGroup<PointerArith>;
 def ext_sizeof_alignof_void_type : Extension<
   "invalid application of '%select{sizeof|alignof|vec_step}0' to a void "
   "type">, InGroup<PointerArith>;

lib/Sema/SemaChecking.cpp

for (unsigned i = 0; i != NumFixed; ++i) {
     // pass in 42.  The 42 gets converted to char.  This is even more strange
     // for things like 45.123 -> char, etc.
     // FIXME: Do this check.
     TheCall->setArg(i+1, Arg.get());
   }
 
   ASTContext& Context = this->getASTContext();
 
+  // Warn if the requested atomic will expand into a library call, as this can
+  // impact performance significantly if it requires kernel intervention or
+  // some other shared but hidden lock.
+  if (Context.getTypeSizeInChars(ValType) >
+      Context.toCharUnitsFromBits(
+          Context.getTargetInfo().getMaxAtomicInlineWidth())) {
+    Diag(DRE->getLocStart(), diag::warn_atomic_builtin_expands_to_libcall);
+  }
+
   // Create a new DeclRefExpr to refer to the new decl.
   DeclRefExpr* NewDRE = DeclRefExpr::Create(
       Context,
       DRE->getQualifierLoc(),
       SourceLocation(),
       NewBuiltinDecl,
       /*enclosing*/ false,
       DRE->getLocation(),

test/Sema/atomic-libcall.c

+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=mips-mti-linux-gnu -Watomic-libcall
+
+// Test that larger than word size atomics are warned about.
+
+long long var;
+
+void foo(long long a) {
+  __sync_fetch_and_add(&var, 0, a); // expected-warning {{atomic builtin expands to library call}}
+}