[PATCH] D37300: [WebAssembly] Add target feature for atomics

[Jacob Gravelle via Phabricator via llvm-commits]

jgravelle-google accepted this revision.
jgravelle-google added a comment.
This revision is now accepted and ready to land.

Nits aside lgtm



================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrAtomics.td:15
 
-// TODO: Implement atomic instructions.
-
-//===----------------------------------------------------------------------===//
-// Atomic fences
-//===----------------------------------------------------------------------===//
-
-// TODO: add atomic fences here...
+let Defs = [ARGUMENTS] in {
 
----------------
Should this surround the `//==--- Atomic loads ---===//` header? Looks odd to me that it starts outside it, but ends inside it.


================
Comment at: test/CodeGen/WebAssembly/atomics.ll:16
+
+define i32 @ldi32atomic(i32 *%p) {
+  %v = load atomic i32, i32* %p seq_cst, align 4
----------------
This would probably be easier to read as `@load_i32_atomic` or something, especially as we get more atomic operations.


https://reviews.llvm.org/D37300