[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: cfe-commits, erichkeane, rsmith.
Herald added subscribers: jdoerfert, jfb.
Herald added a project: clang.

Clang considers the type of a statement expression that returns the value of an _Atomic(ty) to be atomic, and this makes the statement expression incompatible with other expressions of type ty, for example in assignment or comparison.  This patch drops the atomic attribute before creating the StmtExpr.

I noticed that unlike gcc, clang doesn't allow StmtExpr on the left hand side of an assignment, I agree with clang.

I checked with Clark Nelson about the legitimacy of dropping the atomicity, he said "Atomicity has to do with the way data is transferred between a processor and its memory. Once it has been loaded, it's just data."


Repository:
  rC Clang

https://reviews.llvm.org/D59307

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/atomic-expr-stmt.c


Index: test/Sema/atomic-expr-stmt.c
===================================================================
--- /dev/null
+++ test/Sema/atomic-expr-stmt.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm < %s | FileCheck %s
+void test_assign(_Atomic int a, int b) {
+  //CHECK: define void @test_assign(i32 %a, i32 %b)
+  // assignment is OK
+  b = ({a;});
+  //CHECK: %atomic-load = load atomic i32, i32* %a.addr seq_cst, align 4
+  //CHECK: store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
+  //CHECK: %0 = load i32, i32* %tmp, align 4
+  //CHECK: store i32 %0, i32* %b.addr, align 4
+}
+
+int test_compare(_Atomic int x, int y) {
+  //CHECK: define i32 @test_compare(i32 %x, i32 %y)
+  // comparison is OK
+  //CHECK: %0 = load i32, i32* %y.addr, align 4
+  //CHECK: %atomic-load = load atomic i32, i32* %x.addr seq_cst, align 4
+  //CHECK: store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
+  //CHECK: %1 = load i32, i32* %tmp, align 4
+  //CHECK: %cmp = icmp slt i32 %0, %1
+  if (y <({x;}))
+    return 0;
+  else return 1;
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13375,6 +13375,8 @@
       if (const Expr *Value = LastStmt->getExprStmt()) {
         StmtExprMayBindToTemp = true;
         Ty = Value->getType();
+        if (const AtomicType *AtomicRHS = Ty->getAs<AtomicType>())
+          Ty = AtomicRHS->getValueType();
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59307.190438.patch
Type: text/x-patch
Size: 1512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190313/fed3044c/attachment.bin>