Add the new @llvm.nacl.atomic.fence.all intrinsic

This is a follow-up to:
  https://codereview.chromium.org/22240002/
And requires the Clang changes from:
  https://codereview.chromium.org/22294002/

This new intrinsic represents ``asm("":::"~{memory}")`` as well as ``__sync_synchronize()``, and in IR it corresponds to a sequentially-consistent fence surrounded by ``call void asm sideeffect "", "~{memory}"()``.

R=jvoung@chromium.org
TEST= ninja check-all
BUG= https://code.google.com/p/nativeclient/issues/detail?id=3475

Review URL: https://codereview.chromium.org/22474008

4 files changed, 76 insertions, 15 deletions

diff --git a/test/NaCl/PNaClABI/intrinsics.ll b/test/NaCl/PNaClABI/intrinsics.ll
index a210491823..736de63c59 100644
--- a/test/NaCl/PNaClABI/intrinsics.ll
+++ b/test/NaCl/PNaClABI/intrinsics.ll
@@ -54,6 +54,7 @@ declare i16 @llvm.nacl.atomic.cmpxchg.i16(i16*, i16, i16, i32, i32)
 declare i32 @llvm.nacl.atomic.cmpxchg.i32(i32*, i32, i32, i32, i32)
 declare i64 @llvm.nacl.atomic.cmpxchg.i64(i64*, i64, i64, i32, i32)
 declare void @llvm.nacl.atomic.fence(i32)
+declare void @llvm.nacl.atomic.fence.all()
 declare i1 @llvm.nacl.atomic.is.lock.free(i32, i8*)
 
 declare i16 @llvm.bswap.i16(i16)
diff --git a/test/Transforms/NaCl/atomics.ll b/test/Transforms/NaCl/atomics.ll
index b124241062..96b2b7d6e1 100644
--- a/test/Transforms/NaCl/atomics.ll
+++ b/test/Transforms/NaCl/atomics.ll
@@ -1,4 +1,4 @@
-; RUN: opt -nacl-rewrite-atomics -S < %s | FileCheck %s
+; RUN: opt -nacl-rewrite-atomics -remove-asm-memory -S < %s | FileCheck %s
 
 ; Each of these tests validates that the corresponding legacy GCC-style
 ; builtins are properly rewritten to NaCl atomic builtins. Only the
@@ -210,11 +210,44 @@ define i64 @test_val_compare_and_swap_i64(i64* %ptr, i64 %oldval, i64 %newval) {
   ret i64 %res
 }
 
+; This patterns gets emitted by C11/C++11 atomic thread fences.
+;
+; CHECK: @test_c11_fence
+define void @test_c11_fence() {
+  ; CHECK-NEXT: call void @llvm.nacl.atomic.fence(i32 6)
+  ; CHECK-NEXT: ret void
+  fence seq_cst
+  ret void
+}
+
+; This pattern gets emitted for ``__sync_synchronize`` and
+; ``asm("":::"memory")`` when Clang is configured for NaCl.
+;
 ; CHECK: @test_synchronize
 define void @test_synchronize() {
-  ; CHECK-NEXT: call void @llvm.nacl.atomic.fence(i32 6)
+  ; CHECK-NEXT: call void @llvm.nacl.atomic.fence.all()
   ; CHECK-NEXT: ret void
+  call void asm sideeffect "", "~{memory}"()
+  fence seq_cst
+  call void asm sideeffect "", "~{memory}"()
+  ret void
+}
+
+; Make sure the above pattern is respected and not partially-matched.
+;
+; CHECK: @test_synchronize_bad1
+define void @test_synchronize_bad1() {
+  ; CHECK-NOT: call void @llvm.nacl.atomic.fence.all()
+  call void asm sideeffect "", "~{memory}"()
+  fence seq_cst
+  ret void
+}
+
+; CHECK: @test_synchronize_bad2
+define void @test_synchronize_bad2() {
+  ; CHECK-NOT: call void @llvm.nacl.atomic.fence.all()
   fence seq_cst
+  call void asm sideeffect "", "~{memory}"()
   ret void
 }
 
diff --git a/test/Transforms/NaCl/rewrite-asm-memory.ll b/test/Transforms/NaCl/remove-asm-memory.ll
index 8481c831ee..ae799a7459 100644
--- a/test/Transforms/NaCl/rewrite-asm-memory.ll
+++ b/test/Transforms/NaCl/remove-asm-memory.ll
@@ -1,24 +1,37 @@
-; RUN: opt < %s -rewrite-asm-directives -S | FileCheck %s
-; RUN: opt < %s -O3 -rewrite-asm-directives -S | FileCheck %s
-; RUN: opt < %s -O3 -rewrite-asm-directives -S | FileCheck %s -check-prefix=ELIM
-; RUN: opt < %s -rewrite-asm-directives -S | FileCheck %s -check-prefix=CLEANED
+; RUN: opt < %s -nacl-rewrite-atomics -remove-asm-memory -S | \
+; RUN:       FileCheck %s
+; RUN: opt < %s -O3 -nacl-rewrite-atomics -remove-asm-memory -S | \
+; RUN:       FileCheck %s
+; RUN: opt < %s -O3 -nacl-rewrite-atomics -remove-asm-memory -S | \
+; RUN:       FileCheck %s -check-prefix=ELIM
+; RUN: opt < %s -nacl-rewrite-atomics -remove-asm-memory -S | \
+; RUN:       FileCheck %s -check-prefix=CLEANED
 
-; Test that asm("":::"memory"), a compiler barrier, gets rewritten to a
-; sequentially-consistent fence. The test is also run at O3 to make sure
-; that loads and stores don't get unexpectedly eliminated.
+; ``asm("":::"memory")`` is used as a compiler barrier and the GCC-style
+; builtin ``__sync_synchronize`` is intended as a barrier for all memory
+; that could be observed by external threads. They both get rewritten
+; for NaCl by Clang to a sequentially-consistent fence surrounded by
+; ``call void asm sideeffect "", "~{memory}"``.
+;
+; The test is also run at O3 to make sure that non-volatile and
+; non-atomic loads and stores to escaping objects (i.e. loads and stores
+; which could be observed by other threads) don't get unexpectedly
+; eliminated.
 
 ; CLEANED-NOT: asm
 
+target datalayout = "p:32:32:32"
+
 @a = external global i32
 @b = external global i32
 
-; Different triples encode "touch everything" constraints differently.
+; Different triples encode ``asm("":::"memory")``'s "touch everything"
+; constraints differently.  They should get detected and removed.
 define void @memory_assembly_encoding_test() {
 ; CHECK: @memory_assembly_encoding_test()
   call void asm sideeffect "", "~{memory}"()
   call void asm sideeffect "", "~{memory},~{dirflag},~{fpsr},~{flags}"()
-  ; CHECK-NEXT: fence seq_cst
-  ; CHECK-NEXT: fence seq_cst
+  call void asm sideeffect "", "~{foo},~{memory},~{bar}"()
 
   ret void
   ; CHECK-NEXT: ret void
@@ -29,9 +42,11 @@ define void @memory_assembly_ordering_test() {
   %1 = load i32* @a, align 4
   store i32 %1, i32* @b, align 4
   call void asm sideeffect "", "~{memory}"()
+  fence seq_cst
+  call void asm sideeffect "", "~{memory}"()
   ; CHECK-NEXT: %1 = load i32* @a, align 4
   ; CHECK-NEXT: store i32 %1, i32* @b, align 4
-  ; CHECK-NEXT: fence seq_cst
+  ; CHECK-NEXT: call void @llvm.nacl.atomic.fence.all()
 
   ; Redundant load from the previous location, and store to the same
   ; location (making the previous one dead). Shouldn't get eliminated
@@ -39,9 +54,11 @@ define void @memory_assembly_ordering_test() {
   %2 = load i32* @a, align 4
   store i32 %2, i32* @b, align 4
   call void asm sideeffect "", "~{memory}"()
+  fence seq_cst
+  call void asm sideeffect "", "~{memory}"()
   ; CHECK-NEXT: %2 = load i32* @a, align 4
   ; CHECK-NEXT: store i32 %2, i32* @b, align 4
-  ; CHECK-NEXT: fence seq_cst
+  ; CHECK-NEXT: call void @llvm.nacl.atomic.fence.all()
 
   ; Same here.
   %3 = load i32* @a, align 4
diff --git a/test/Transforms/NaCl/resolve-pnacl-intrinsics.ll b/test/Transforms/NaCl/resolve-pnacl-intrinsics.ll
index 737561ee1d..0a297057fe 100644
--- a/test/Transforms/NaCl/resolve-pnacl-intrinsics.ll
+++ b/test/Transforms/NaCl/resolve-pnacl-intrinsics.ll
@@ -29,6 +29,7 @@ declare i16 @llvm.nacl.atomic.cmpxchg.i16(i16*, i16, i16, i32, i32)
 declare i32 @llvm.nacl.atomic.cmpxchg.i32(i32*, i32, i32, i32, i32)
 declare i64 @llvm.nacl.atomic.cmpxchg.i64(i64*, i64, i64, i32, i32)
 declare void @llvm.nacl.atomic.fence(i32)
+declare void @llvm.nacl.atomic.fence.all()
 declare i1 @llvm.nacl.atomic.is.lock.free(i32, i8*)
 
 ; These declarations must be here because the function pass expects
@@ -93,10 +94,19 @@ define i32 @test_val_compare_and_swap_i32(i32* %ptr, i32 %oldval, i32 %newval) {
   ret i32 %1
 }
 
+; CHECK: @test_c11_fence
+define void @test_c11_fence() {
+  ; CHECK: fence seq_cst
+  call void @llvm.nacl.atomic.fence(i32 6)
+  ret void
+}
+
 ; CHECK: @test_synchronize
 define void @test_synchronize() {
+  ; CHECK: call void asm sideeffect "", "~{memory}"()
   ; CHECK: fence seq_cst
-  call void @llvm.nacl.atomic.fence(i32 6)
+  ; CHECK: call void asm sideeffect "", "~{memory}"()
+  call void @llvm.nacl.atomic.fence.all()
   ret void
 }