[clang] improve diagnostics for misaligned and large atomics

[tschuett]

Summary

"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.

[xbolva00]

clang/include/clang/Basic/DiagnosticFrontendKinds.td

274

Use ; rather than ( ... ) ?

[tschuett]

Replaced (...) by ;

[tschuett]

introduced in https://reviews.llvm.org/D45319

[xbolva00]

Thanks. Can you add some test cases?

[tschuett]

add/updated test

[jfb]

clang/test/CodeGen/atomics-sema-alignment.c

43 ↗

(On Diff #282485)

"the expected (16 bytes)" should be "the expected alignment (16 bytes)"

[tschuett]

I wanted to save space. I know that the alignment is missing there, but the line is already too long.

I have no rights to commit.

[tschuett]

added missing "alignment"

[tschuett]

I would appreciate it, if somebody could commit this patch on my behalf.
Thorsten Schuett
[e-mail]

Thanks.

clang/include/clang/Basic/DiagnosticFrontendKinds.td

 def err_duplicate_mangled_name : Error<
   "definition with same mangled name '%0' as another definition">;
 def err_cyclic_alias : Error<
   "%select{alias|ifunc}0 definition is part of a cycle">;
 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 (%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 "
   "as the %select{aliasee|resolver}2">,
   InGroup<IgnoredAttributes>;
 
 let CategoryName = "Instrumentation Issue" in {
 def warn_profile_data_out_of_date : Warning<

clang/include/clang/Basic/DiagnosticGroups.td

 def UsedButMarkedUnused : DiagGroup<"used-but-marked-unused">;
 def UserDefinedLiterals : DiagGroup<"user-defined-literals">;
 def UserDefinedWarnings : DiagGroup<"user-defined-warnings">;
 def ReorderCtor : DiagGroup<"reorder-ctor">;
 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]>;
 def ARCUnsafeRetainedAssign : DiagGroup<"arc-unsafe-retained-assign">;
 def ARCRetainCycles : DiagGroup<"arc-retain-cycles">;
 def ARCNonPodMemAccess : DiagGroup<"arc-non-pod-memaccess">;
 def AutomaticReferenceCounting : DiagGroup<"arc",
                                            [ARCUnsafeRetainedAssign,

clang/lib/CodeGen/CGAtomic.cpp

RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   CharUnits sizeChars, alignChars;
   std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(AtomicTy);
   uint64_t Size = sizeChars.getQuantity();
   unsigned MaxInlineWidthInBits = getTarget().getMaxAtomicInlineWidth();
 
   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());
   llvm::Value *Scope =
       E->getScopeModel() ? EmitScalarExpr(E->getScope()) : nullptr;
 
   switch (E->getOp()) {
   case AtomicExpr::AO__c11_atomic_init: