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)">;
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)">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
