From e18c6ef6b41a59af73bf5c3d7d52a8c53a471e5d Mon Sep 17 00:00:00 2001 From: Thorsten Schuett Date: Tue, 4 Aug 2020 11:10:01 -0700 Subject: [PATCH] [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 --- .../include/clang/Basic/DiagnosticFrontendKinds.td | 12 +++++++-- clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/lib/CodeGen/CGAtomic.cpp | 16 +++++++++--- clang/test/CodeGen/atomics-sema-alignment.c | 29 +++++++++++++++++++--- 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index b202d2a..6434d92 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>; + "misaligned atomic operation may incur " + "significant performance penalty" + "; the expected alignment (%0 bytes) exceeds the actual alignment (%1 bytes)">, + InGroup; + +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; 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 1e829be..be62461f 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 a58450d..b7ada4a 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 9443af3..d0058f1 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)}} +} -- 2.7.4