Concurrency support for PNaCl ABI

test/NaCl/PNaClABI/intrinsics.ll

Index: test/NaCl/PNaClABI/intrinsics.ll
diff --git a/test/NaCl/PNaClABI/intrinsics.ll b/test/NaCl/PNaClABI/intrinsics.ll
index 66113061abbc6990c09f86d21f13b56fe49633ee..7db61a9b5062d713c00b9a2aff696e2c2e26ec49 100644
--- a/test/NaCl/PNaClABI/intrinsics.ll
+++ b/test/NaCl/PNaClABI/intrinsics.ll
@@ -25,6 +25,41 @@ declare void @llvm.memcpy.p0i8.p0i8.i64(i8* %dest, i8* %src,
 ; CHECK-NOT: Function llvm.nacl.read.tp is a disallowed LLVM intrinsic
 declare i8* @llvm.nacl.read.tp()
 
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.load.i8 is a disallowed LLVM intrinsic

[Mark Seaborn, 2013/07/02 19:16:02]

These just become a single "CHECK-NOT: disallowed" after you rebase.

+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.load.i16 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.load.i32 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.load.i64 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.store.i8 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.store.i16 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.store.i32 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.store.i64 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.rmw.i8 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.rmw.i16 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.rmw.i32 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.rmw.i64 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.cmpxchg.i8 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.cmpxchg.i16 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.cmpxchg.i32 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.cmpxchg.i64 is a disallowed LLVM intrinsic
+; CHECK-NOT: Function llvm.nacl.atomic.llvm.nacl.atomic.fence is a disallowed LLVM intrinsic
+declare i8 @llvm.nacl.atomic.load.i8(i8*, i32)
+declare i16 @llvm.nacl.atomic.load.i16(i16*, i32)
+declare i32 @llvm.nacl.atomic.load.i32(i32*, i32)
+declare i64 @llvm.nacl.atomic.load.i64(i64*, i32)

[Mark Seaborn, 2013/07/02 19:16:02]

I still disagree with allowing 64-bit atomics.  These simply don't work with
MIPS in the presence of shared memory.  Please can we leave 64-bit atomics out
in the first version?

[JF, 2013/07/02 23:14:54]

Without shared memory there is no issue, even on MIPS: we offer the same
guarantees as those of C11/C++11.

With shared memory on MIPS (which is an issue that you only brought up today
AFAICT, and have not documented anywhere) there is also no issue:
 - A similar problem already exists with HTML5 filesystem.
 - Offering 64-bit atomics at least allows things to work in some settings,
instead of forcing users to first rewrite their code and recompile with vanilla
load/store and not express any intent at all (which would prevent us from ever
solving the issue for pexes that are in the wild).
 - The 64-bit atomic problem is equivalent to futex support on shared memory. It
is no more work to support, and has the same solution.

For now we don't have shared memory. Once we do the is_lock_free primitive will
be used to determine if we support atomics on shared memory or not. It will also
be possible to implement this 64-bit global lock either through futex emulation,
or by mapping the 64-bit global lock on all processes that share memory. I will
update the PNaClLangRef to that effect.

For now we don't have shared memory and we don't support MIPS, and in the future
we have a clear technical solution that works. 64-bit atomics are therefore a
non-issue and will be supported in the first release.

I encourage you to file a bug and mention this discussion.

I've discussed this with sehr and bsy and have their agreement.

[Mark Seaborn, 2013/07/02 23:34:10]

That's not the case, because the Javascript interface does not provide atomic
memory operations on files.
Having things work in some settings is the problem: programs will potentially
work on x86 and ARM, but then fail on MIPS.  That is the kind of problem we've
been trying to avoid, because it breaks PNaCl's portability guarantee.
It's not equivalent, because NaCl futexes are defined to be per-process. 
There's no way to use any of NaCl's current futex interfaces such that a futex
will have cross-process behaviour on a shared memory page, so there's no way
that a program will accidentally depend on such cross-process behaviour.
That would also break PNaCl's portability guarantee, if a program can query
is_lock_free and get different answers for 64-bit on x86 and MIPS.
I'm not sure what you're proposing there?

[JF, 2013/07/02 23:50:57]

I suggest you read the updated PNaClLangRef, it should contain the information
required for the ABI and users (I'll split these documentation sources later).
Please let me know if that document is unclear.

+declare void @llvm.nacl.atomic.store.i8(i8, i8*, i32)
+declare void @llvm.nacl.atomic.store.i16(i16, i16*, i32)
+declare void @llvm.nacl.atomic.store.i32(i32, i32*, i32)
+declare void @llvm.nacl.atomic.store.i64(i64, i64*, i32)
+declare i8 @llvm.nacl.atomic.rmw.i8(i32, i8*, i8, i32)
+declare i16 @llvm.nacl.atomic.rmw.i16(i32, i16*, i16, i32)
+declare i32 @llvm.nacl.atomic.rmw.i32(i32, i32*, i32, i32)
+declare i64 @llvm.nacl.atomic.rmw.i64(i32, i64*, i64, i32)
+declare i8 @llvm.nacl.atomic.cmpxchg.i8(i8*, i8, i8, i32, i32)
+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)
+
 ; CHECK-NOT: Function llvm.bswap.i16 is a disallowed LLVM intrinsic
 declare i16 @llvm.bswap.i16(i16)