[PATCH] D12002: Initial WebAssembly support in clang

JF Bastien via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 14 13:55:52 PDT 2015


jfb added inline comments.

================
Comment at: lib/Basic/Targets.cpp:6938
@@ +6937,3 @@
+    BigEndian = false;
+    NoAsmVariants = true;
+    SuitableAlign = 128;
----------------
True, leaving it as is sgtm.

================
Comment at: lib/Basic/Targets.cpp:6994
@@ +6993,3 @@
+#include "clang/Basic/BuiltinsWebAssembly.def"
+};
+
----------------
Weird, the comment moved around. I put it on `WebAssembly32TargetInfo` which doesn't seem subclassed? Same for 64. Or is that going to be on the LLVM side? `final` on `getTargetDefines` instead?

================
Comment at: test/Preprocessor/init.c:8478
@@ +8477,3 @@
+// WEBASSEMBLY32:#define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_SHORT_LOCK_FREE 2
----------------
Yes, we don't know yet whether we guarantee atomicity so returning "maybe" is the conservative thing (which we can change in the future). We can discuss what we guarantee on the spec side, but for now the conservative thing is better IMO (and changing it isn't a problem, whereas changing "always" is a problem).

FWIW [N4509](http://wg21.link/n4509) is relevant.


Repository:
  rL LLVM

http://reviews.llvm.org/D12002





More information about the cfe-commits mailing list