[clang] e18c6ef - [clang] improve diagnostics for misaligned and large atomics

[JF Bastien via cfe-commits]

Author: Thorsten Schuett
Date: 2020-08-04T11:10:29-07:00
New Revision: e18c6ef6b41a59af73bf5c3d7d52a8c53a471e5d

URL: https://github.com/llvm/llvm-project/commit/e18c6ef6b41a59af73bf5c3d7d52a8c53a471e5d
DIFF: https://github.com/llvm/llvm-project/commit/e18c6ef6b41a59af73bf5c3d7d52a8c53a471e5d.diff

LOG: [clang] improve diagnostics for misaligned and large atomics

"Listing the alignment and access size (== expected alignment) in the warning
seems like a good idea."

solves PR 46947

  struct Foo {
    struct Bar {
      void * a;
      void * b;
    };
    Bar bar;
  };

  struct ThirtyTwo {
    struct Large {
      void * a;
      void * b;
      void * c;
      void * d;
    };
    Large bar;
  };

  void braz(Foo *foo, ThirtyTwo *braz) {
    Foo::Bar bar;
    __atomic_load(&foo->bar, &bar, __ATOMIC_RELAXED);

    ThirtyTwo::Large foobar;
    __atomic_load(&braz->bar, &foobar, __ATOMIC_RELAXED);
  }

repro.cpp:21:3: warning: misaligned atomic operation may incur significant performance penalty; the expected (16 bytes) exceeds the actual alignment (8 bytes) [-Watomic-alignment]
  __atomic_load(&foo->bar, &bar, __ATOMIC_RELAXED);
  ^
repro.cpp:24:3: warning: misaligned atomic operation may incur significant performance penalty; the expected (32 bytes) exceeds the actual alignment (8 bytes) [-Watomic-alignment]
  __atomic_load(&braz->bar, &foobar, __ATOMIC_RELAXED);
  ^
repro.cpp:24:3: warning: large atomic operation may incur significant performance penalty; the access size (32 bytes) exceeds the max lock-free size (16  bytes) [-Watomic-alignment]
3 warnings generated.

Differential Revision: https://reviews.llvm.org/D85102

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticFrontendKinds.td
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/lib/CodeGen/CGAtomic.cpp
    clang/test/CodeGen/atomics-sema-alignment.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index b202d2abffa0..6434d92fd8fc 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -270,8 +270,16 @@ def err_ifunc_resolver_return : Error<
   "ifunc resolver function must return a pointer">;
 
 def warn_atomic_op_misaligned : Warning<
-  "%select{large|misaligned}0 atomic operation may incur "
-  "significant performance penalty">, InGroup<DiagGroup<"atomic-alignment">>;
+  "misaligned atomic operation may incur "
+  "significant performance penalty"
+  "; the expected alignment (%0 bytes) exceeds the actual alignment (%1 bytes)">,
+  InGroup<AtomicAlignment>;
+
+def warn_atomic_op_oversized : Warning<
+  "large atomic operation may incur "
+  "significant performance penalty"
+  "; the access size (%0 bytes) exceeds the max lock-free size (%1  bytes)">,
+InGroup<AtomicAlignment>;
 
 def warn_alias_with_section : Warning<
   "%select{alias|ifunc}1 will not be in section '%0' but in the same section "

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 1e829be4028e..be62461faef4 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -699,6 +699,7 @@ def ReorderInitList : DiagGroup<"reorder-init-list">;
 def Reorder : DiagGroup<"reorder", [ReorderCtor, ReorderInitList]>;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def AtomicAlignment : DiagGroup<"atomic-alignment">;
 def CustomAtomic : DiagGroup<"custom-atomic-properties">;
 def AtomicProperties : DiagGroup<"atomic-properties",
                                  [ImplicitAtomic, CustomAtomic]>;

diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index a58450ddd4c5..b7ada4ac7e3b 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -807,10 +807,20 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   bool Oversized = getContext().toBits(sizeChars) > MaxInlineWidthInBits;
   bool Misaligned = (Ptr.getAlignment() % sizeChars) != 0;
   bool UseLibcall = Misaligned | Oversized;
+  CharUnits MaxInlineWidth =
+      getContext().toCharUnitsFromBits(MaxInlineWidthInBits);
 
-  if (UseLibcall) {
-    CGM.getDiags().Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
-        << !Oversized;
+  DiagnosticsEngine &Diags = CGM.getDiags();
+
+  if (Misaligned) {
+    Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_misaligned)
+        << (int)sizeChars.getQuantity()
+        << (int)Ptr.getAlignment().getQuantity();
+  }
+
+  if (Oversized) {
+    Diags.Report(E->getBeginLoc(), diag::warn_atomic_op_oversized)
+        << (int)sizeChars.getQuantity() << (int)MaxInlineWidth.getQuantity();
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());

diff  --git a/clang/test/CodeGen/atomics-sema-alignment.c b/clang/test/CodeGen/atomics-sema-alignment.c
index 9443af354ec5..d0058f1da8b0 100644
--- a/clang/test/CodeGen/atomics-sema-alignment.c
+++ b/clang/test/CodeGen/atomics-sema-alignment.c
@@ -12,10 +12,10 @@ 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}}
+  __atomic_load(p, &res, 0);                    // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_store(p, &res, 0);                   // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (8 bytes) exceeds the actual alignment (4 bytes)}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (4 bytes) exceeds the actual alignment (1 bytes)}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (4 bytes) exceeds the actual alignment (1 bytes)}}
 }
 
 void func1(LongStruct *p) {
@@ -25,3 +25,24 @@ void func1(LongStruct *p) {
   __atomic_fetch_add((int *)p, 1, 2);
   __atomic_fetch_sub((int *)p, 1, 3);
 }
+
+typedef struct {
+  void *a;
+  void *b;
+} Foo;
+
+typedef struct {
+  void *a;
+  void *b;
+  void *c;
+  void *d;
+} __attribute__((aligned(32))) ThirtyTwo;
+
+void braz(Foo *foo, ThirtyTwo *braz) {
+  Foo bar;
+  __atomic_load(foo, &bar, __ATOMIC_RELAXED); // expected-warning {{misaligned atomic operation may incur significant performance penalty; the expected alignment (16 bytes) exceeds the actual alignment (8 bytes)}}
+
+  ThirtyTwo thirtyTwo1;
+  ThirtyTwo thirtyTwo2;
+  __atomic_load(&thirtyTwo1, &thirtyTwo2, __ATOMIC_RELAXED); // expected-warning {{large atomic operation may incur significant performance penalty; the access size (32 bytes) exceeds the max lock-free size (16  bytes)}}
+}