From: Tim Northover <tnorthover@apple.com>
Date: Mon, 23 Apr 2018 08:16:24 +0000 (+0000)
Subject: [Atomics] warn about atomic accesses using libcalls
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e89759088e133b5533e9d2b1eaa6112e92dcd9b4;p=clang

[Atomics] warn about atomic accesses using libcalls

If an atomic variable is misaligned (and that suspicion is why Clang emits
libcalls at all) the runtime support library will have to use a lock to safely
access it, with potentially very bad performance consequences. There's a very
good chance this is unintentional so it makes sense to issue a warning.

Also give it a named group so people can promote it to an error, or disable it
if they really don't care.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@330566 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index d2a484c415..152879cac3 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7111,6 +7111,9 @@ def warn_atomic_op_has_invalid_memory_order : Warning<
   InGroup<DiagGroup<"atomic-memory-ordering">>;
 def err_atomic_op_has_invalid_synch_scope : Error<
   "synchronization scope argument to atomic operation is invalid">;
+def warn_atomic_op_misaligned : Warning<
+  "misaligned or large atomic operation may incur significant performance penalty">,
+  InGroup<DiagGroup<"atomic-alignment">>;
 
 def err_overflow_builtin_must_be_int : Error<
   "operand argument to overflow builtin must be an integer (%0 invalid)">;
diff --git a/lib/CodeGen/CGAtomic.cpp b/lib/CodeGen/CGAtomic.cpp
index efc708988a..47a82b602a 100644
--- a/lib/CodeGen/CGAtomic.cpp
+++ b/lib/CodeGen/CGAtomic.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
@@ -751,6 +752,13 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   Address Dest = Address::invalid();
   Address Ptr = EmitPointerWithAlignment(E->getPtr());
 
+  if (E->getOp() == AtomicExpr::AO__c11_atomic_init ||
+      E->getOp() == AtomicExpr::AO__opencl_atomic_init) {
+    LValue lvalue = MakeAddrLValue(Ptr, AtomicTy);
+    EmitAtomicInit(E->getVal1(), lvalue);
+    return RValue::get(nullptr);
+  }
+
   CharUnits sizeChars, alignChars;
   std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(AtomicTy);
   uint64_t Size = sizeChars.getQuantity();
@@ -758,12 +766,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   bool UseLibcall = ((Ptr.getAlignment() % sizeChars) != 0 ||
                      getContext().toBits(sizeChars) > MaxInlineWidthInBits);
 
-  if (E->getOp() == AtomicExpr::AO__c11_atomic_init ||
-      E->getOp() == AtomicExpr::AO__opencl_atomic_init) {
-    LValue lvalue = MakeAddrLValue(Ptr, AtomicTy);
-    EmitAtomicInit(E->getVal1(), lvalue);
-    return RValue::get(nullptr);
-  }
+  if (UseLibcall)
+    CGM.getDiags().Report(E->getLocStart(), diag::warn_atomic_op_misaligned);
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
   llvm::Value *Scope =
diff --git a/test/CodeGen/atomics-sema-alignment.c b/test/CodeGen/atomics-sema-alignment.c
new file mode 100644
index 0000000000..364743dd53
--- /dev/null
+++ b/test/CodeGen/atomics-sema-alignment.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu %s -emit-llvm -o /dev/null -verify
+
+typedef struct {
+  int a, b;
+} IntPair;
+
+typedef struct {
+  long long a;
+} LongStruct;
+
+typedef int __attribute__((aligned(1))) unaligned_int;
+
+void func(IntPair *p) {
+  IntPair res;
+  __atomic_load(p, &res, 0); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+  __atomic_store(p, &res, 0); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+}
+
+void func1(LongStruct *p) {
+  LongStruct res;
+  __atomic_load(p, &res, 0);
+  __atomic_store(p, &res, 0);
+  __atomic_fetch_add((int *)p, 1, 2);
+  __atomic_fetch_sub((int *)p, 1, 3);
+}