]> granicus.if.org Git - clang/commitdiff
_Atomic of empty struct shouldn't assert
authorJF Bastien <jfb@google.com>
Wed, 9 May 2018 03:51:12 +0000 (03:51 +0000)
committerJF Bastien <jfb@google.com>
Wed, 9 May 2018 03:51:12 +0000 (03:51 +0000)
Summary:

An _Atomic of an empty struct is pretty silly. In general we just widen empty
structs to hold a byte's worth of storage, and we represent size and alignment
as 0 internally and let LLVM figure out what to do. For _Atomic it's a bit
different: the memory model mandates concrete effects occur when atomic
operations occur, so in most cases actual instructions need to get emitted. It's
really not worth trying to optimize empty struct atomics by figuring out e.g.
that a fence would do, even though sane compilers should do optimize atomics.
Further, wg21.link/p0528 will fix C++20 atomics with padding bits so that
cmpxchg on them works, which means that we'll likely need to do the zero-init
song and dance for empty atomic structs anyways (and I think we shouldn't
special-case this behavior to C++20 because prior standards are just broken).

This patch therefore makes a minor change to r176658 "Promote atomic type sizes
up to a power of two": if the width of the atomic's value type is 0, just use 1
byte for width and leave alignment as-is (since it should never be zero, and
over-aligned zero-width structs are weird but fine).

This fixes an assertion:
   (NumBits >= MIN_INT_BITS && "bitwidth too small"), function get, file ../lib/IR/Type.cpp, line 241.

It seems like this has run into other assertions before (namely the unreachable
Kind check in ImpCastExprToType), but I haven't reproduced that issue with
tip-of-tree.

<rdar://problem/39678063>

Reviewers: arphaman, rjmccall

Subscribers: aheejin, cfe-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@331845 91177308-0d34-0410-b5e6-96231b3b80d8

lib/AST/ASTContext.cpp
test/CodeGen/c11atomics-ios.c
test/CodeGen/c11atomics.c

index f8de2b44809fca5a70d7c9dec44908943c6190ef..0f76136e35113f2fcfca8a5524b65780b7824fcb 100644 (file)
@@ -1958,10 +1958,16 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
     Width = Info.Width;
     Align = Info.Align;
 
-    // If the size of the type doesn't exceed the platform's max
-    // atomic promotion width, make the size and alignment more
-    // favorable to atomic operations:
-    if (Width != 0 && Width <= Target->getMaxAtomicPromoteWidth()) {
+    if (!Width) {
+      // An otherwise zero-sized type should still generate an
+      // atomic operation.
+      Width = Target->getCharWidth();
+      assert(Align);
+    } else if (Width <= Target->getMaxAtomicPromoteWidth()) {
+      // If the size of the type doesn't exceed the platform's max
+      // atomic promotion width, make the size and alignment more
+      // favorable to atomic operations:
+
       // Round the size up to a power of 2.
       if (!llvm::isPowerOf2_64(Width))
         Width = llvm::NextPowerOf2(Width);
index 0e24ed228cac2ae07f27ccade1a0f956d63604a8..f48e10e4aa430f514068cd136ac1a1f3cb25283d 100644 (file)
@@ -319,3 +319,19 @@ _Bool test_promoted_cmpxchg(_Atomic(PS) *addr, PS *desired, PS *new) {
 
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty testEmptyStructLoad(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @testEmptyStructLoad(
+  // CHECK-NOT: @__atomic_load
+  // CHECK: load atomic i8, i8* %{{.*}} seq_cst, align 1
+  return *empty;
+}
+
+void testEmptyStructStore(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @testEmptyStructStore(
+  // CHECK-NOT: @__atomic_store
+  // CHECK: store atomic i8 %{{.*}}, i8* %{{.*}} seq_cst, align 1
+  *empty = value;
+}
index 3774ded7c426b3691a91c29f5ba41fbe49c24470..cf251738be55e4a9549fc8f1af030d66f944c4e0 100644 (file)
@@ -474,3 +474,17 @@ _Bool test_promoted_cmpxchg(_Atomic(PS) *addr, PS *desired, PS *new) {
   // CHECK:   ret i1 [[RES]]
   return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
 }
+
+struct Empty {};
+
+struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
+  // CHECK-LABEL: @test_empty_struct_load(
+  // CHECK: call arm_aapcscc zeroext i8 @__atomic_load_1(i8* %{{.*}}, i32 5)
+  return __c11_atomic_load(empty, 5);
+}
+
+void test_empty_struct_store(_Atomic(struct Empty)* empty, struct Empty value) {
+  // CHECK-LABEL: @test_empty_struct_store(
+  // CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %{{.*}}, i32 5)
+  __c11_atomic_store(empty, value, 5);
+}