[PATCH] D12002: Initial WebAssembly support in clang

[Dan Gohman via cfe-commits]

sunfish planned changes to this revision.
sunfish marked 3 inline comments as done.

================
Comment at: lib/Basic/Targets.cpp:6935
@@ +6934,3 @@
+    NoAsmVariants = true;
+    LongDoubleWidth = LongDoubleAlign = 64;
+    SuitableAlign = 128;
----------------
jfb wrote:
> That's already the default?
I thought I'd leave it in while we discuss whether this is really what we want.

================
Comment at: lib/Basic/Targets.cpp:6937
@@ +6936,3 @@
+    SuitableAlign = 128;
+    RegParmMax = 0; // Disallow regparm
+  }
----------------
jfb wrote:
> Already the default?
Right.

================
Comment at: lib/Basic/Targets.cpp:6938
@@ +6937,3 @@
+    RegParmMax = 0; // Disallow regparm
+  }
+
----------------
jfb wrote:
> `TLSSupported = false` for now.
Since the MVP doesn't have threads at all, all variables are TLS :-). And after the MVP, we'll add threads with TLS. Is there a downside to leaving this on for now? It'll make the thread prototyping work easier.

================
Comment at: lib/Basic/Targets.cpp:6941
@@ +6940,3 @@
+  void
+  getDefaultFeatures(llvm::StringMap<bool> &Features) const override final {}
+  bool setCPU(const std::string &Name) override final {
----------------
jfb wrote:
> Not needed, since that's the default impl?
We're going to put code in there before long anyway, so there's no harm in getting it ready.

================
Comment at: lib/Basic/Targets.cpp:6956
@@ +6955,3 @@
+
+    Builder.defineMacro("__REGISTER_PREFIX__", "");
+  }
----------------
jfb wrote:
> I'm not sure we need this. Does it just make porting easier?
I don't know if we specifically need it; I just included it because lots of other popular targets have it.

================
Comment at: lib/Basic/Targets.cpp:6983
@@ +6982,3 @@
+  const char *getClobbers() const override { return ""; }
+  bool isCLZForZeroUndef() const override { return false; }
+};
----------------
jfb wrote:
> `final` for these two.
Right.

================
Comment at: lib/Basic/Targets.cpp:6994
@@ +6993,3 @@
+
+class WebAssembly32TargetInfo : public WebAssemblyTargetInfo {
+public:
----------------
jfb wrote:
> `final`
This class is subclassed.

================
Comment at: lib/Basic/Targets.cpp:7015
@@ +7014,3 @@
+
+class WebAssembly64TargetInfo : public WebAssemblyTargetInfo {
+public:
----------------
jfb wrote:
> `final`
This one is too.

================
Comment at: lib/Driver/Tools.cpp:1559
@@ -1558,1 +1558,3 @@
 
+/// getWebAssemblyTargetCPU - Get the (LLVM) name of the WebAssembly cpu we are
+/// targeting.
----------------
jfb wrote:
> New comment style without `name -`.
Right.

================
Comment at: test/CodeGen/wasm-arguments.c:91
@@ +90,2 @@
+// WEBASSEMBLY64: define void @f10(%struct.bitfield1* byval align 4 %bf1)
+void f10(bitfield1 bf1) {}
----------------
jfb wrote:
> TODO for vararg test?
There's a TODO for "implement va_list properly" elsewhere, so we can add tests when we actually implement it.

================
Comment at: test/Driver/wasm32-unknown-unknown.cpp:51
@@ +50,3 @@
+void __wasm__defined() {}
+#endif
+
----------------
jfb wrote:
> Also test `__wasm32__` (and 64 in the other file).
test/Preprocessor/init.c covers this.

================
Comment at: test/Driver/wasm32-unknown-unknown.cpp:64
@@ +63,3 @@
+void _REENTRANTundefined() {}
+#endif
+
----------------
jfb wrote:
> Test `__REGISTER_PREFIX__` if we do keep it.
test/Preprocessor/init.c covers this.

================
Comment at: test/Preprocessor/init.c:8478
@@ +8477,3 @@
+// WEBASSEMBLY32:#define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_SHORT_LOCK_FREE 2
+// WEBASSEMBLY32:#define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
----------------
jfb wrote:
> `ATOMIC_*_LOCK_FREE` should always be `1` for "maybe".
Clang is setting these automatically based on the value of MaxAtomicInlineWidth. Are you saying 32 is the wrong value for wasm32? This is perhaps a discussion to be had on the spec side.

The value for wasm64 is also an interesting topic for the spec side.


Repository:
  rL LLVM

http://reviews.llvm.org/D12002