[PATCH] D45319: [Atomics] warn about misaligned atomic accesses using libcalls

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
t.p.northover added a reviewer: rsmith.
t.p.northover added a project: clang.
Herald added a subscriber: mcrosier.

If an atomic variable is misaligned Clang will emit accesses to it as a libcall. These calls are likely to be slow and involve locks; they are almost certainly not what the developer intended. So this patch adds a warning (promotable or ignorable) for tat situation.


Repository:
  rC Clang

https://reviews.llvm.org/D45319

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGAtomic.cpp
  clang/test/CodeGen/atomics-sema-alignment.c


Index: clang/test/CodeGen/atomics-sema-alignment.c
===================================================================
--- /dev/null
+++ clang/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 atomic operation may incur significant performance penalty}}
+  __atomic_store(p, &res, 0); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned 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);
+}
Index: clang/lib/CodeGen/CGAtomic.cpp
===================================================================
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/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"
@@ -879,6 +880,8 @@
 
   // Use a library call.  See: http://gcc.gnu.org/wiki/Atomic/GCCMM/LIbrary .
   if (UseLibcall) {
+    CGM.getDiags().Report(E->getLocStart(), diag::warn_atomic_op_misaligned);
+
     bool UseOptimizedLibcall = false;
     switch (E->getOp()) {
     case AtomicExpr::AO__c11_atomic_init:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7087,6 +7087,9 @@
   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 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)">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45319.141145.patch
Type: text/x-patch
Size: 2662 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180405/1b708409/attachment.bin>