mibintc marked 2 inline comments as done.
mibintc added a comment.
Thanks for test suggestions, will follow up
================
Comment at: test/Sema/atomic-expr-stmt.c:7
+ //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
----------------
jfb wrote:
> Why is there a store to `a` here?
Yes the IR is odd, I will rewrite with a as global _Atomic int, not a parameter
================
Comment at: test/Sema/atomic-expr-stmt.c:9
+ //CHECK: %0 = load i32, i32* %tmp, align 4
+ //CHECK: store i32 %0, i32* %b.addr, align 4
+}
----------------
jfb wrote:
> What's in `%tmp`? I'd expect the value loaded from `a` to be stored to `b`.
It does seem like the "store atomic" to tmp shouldn't exist. The %tmp is the
value returned by the StmtExpr. Since the atomic load has already occurred, the
value returned by StmtExpr could just be %atomic-load. If I compare this IR to
a test case without the StmtExpr, there's just the atomic load of a which is
then stored into b. I think the value of the atomic-load is still hanging on
to the atomic attribute but it should be dropped.
I changed the test like this, and the IR follows _Atomic int a; void
test_assign(int b) {
// assignment is OK
b = ({a;});
}
; Function Attrs: noinline nounwind optnone define void @test_assign(i32 %b) #0
{
entry:
%b.addr = alloca i32, align 4
%tmp = alloca i32, align 4
store i32 %b, i32* %b.addr, align 4
%atomic-load = load atomic i32, i32* @a seq_cst, align 4
store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
%0 = load i32, i32* %tmp, align 4
store i32 %0, i32* %b.addr, align 4
ret void
}
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59307/new/
https://reviews.llvm.org/D59307
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
