[PATCH] D12002: Initial WebAssembly support in clang

[Eric Christopher via cfe-commits]

echristo added a subscriber: echristo.
echristo added a comment.

Some inline comments.

Thanks!

-eric


================
Comment at: include/clang/Basic/TargetCXXABI.h:166
@@ -148,1 +165,3 @@
 
+  /// \brief Are member functions differently aligned?
+  bool areMemberFunctionsAligned() const {
----------------
Can you elaborate on the comment here as to what the alignment here means or something? It looks incomplete otherwise.

================
Comment at: lib/Basic/Targets.cpp:6935-6941
@@ +6934,9 @@
+      if (Feature == "+simd128") {
+        SIMDLevel = std::max(SIMDLevel, SIMD128);
+        continue;
+      }
+      if (Feature == "-simd128") {
+        SIMDLevel = std::min(SIMDLevel, SIMDEnum(SIMD128 - 1));
+        continue;
+      }
+
----------------
Might make sense to copy the x86 bits here?

================
Comment at: lib/Basic/Targets.cpp:7633-7658
@@ -7464,1 +7632,28 @@
   }
+  case llvm::Triple::wasm32: {
+    // Until specific variations are defined, don't permit any.
+    if (Triple.getSubArch() != llvm::Triple::NoSubArch ||
+        Triple.getVendor() != llvm::Triple::UnknownVendor ||
+        Triple.getOS() != llvm::Triple::UnknownOS ||
+        Triple.getEnvironment() != llvm::Triple::UnknownEnvironment ||
+        Triple.getObjectFormat() != llvm::Triple::UnknownObjectFormat ||
+        (!Triple.getVendorName().empty() &&
+         Triple.getVendorName() != "unknown") ||
+        (!Triple.getOSName().empty() && Triple.getOSName() != "unknown") ||
+        (Triple.hasEnvironment() && Triple.getEnvironmentName() != "unknown"))
+      return nullptr;
+    return new WebAssemblyOSTargetInfo<WebAssembly32TargetInfo>(Triple);
+  }
+  case llvm::Triple::wasm64: {
+    // Until specific variations are defined, don't permit any.
+    if (Triple.getSubArch() != llvm::Triple::NoSubArch ||
+        Triple.getVendor() != llvm::Triple::UnknownVendor ||
+        Triple.getOS() != llvm::Triple::UnknownOS ||
+        Triple.getEnvironment() != llvm::Triple::UnknownEnvironment ||
+        Triple.getObjectFormat() != llvm::Triple::UnknownObjectFormat ||
+        (!Triple.getVendorName().empty() &&
+         Triple.getVendorName() != "unknown") ||
+        (!Triple.getOSName().empty() && Triple.getOSName() != "unknown") ||
+        (Triple.hasEnvironment() && Triple.getEnvironmentName() != "unknown"))
+      return nullptr;
+    return new WebAssemblyOSTargetInfo<WebAssembly64TargetInfo>(Triple);
----------------
Seems overly complicated? Maybe just a positive test?

================
Comment at: lib/CodeGen/CodeGenModule.cpp:824
@@ +823,3 @@
+  if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
+    // C++ ABI requires 2-byte alignment for member functions.
+    if (F->getAlignment() < 2 && isa<CXXMethodDecl>(D))
----------------
Update the comment?

================
Comment at: lib/Driver/ToolChains.cpp:3853-3873
@@ +3852,23 @@
+
+bool WebAssembly::IsMathErrnoDefault() const { return false; }
+
+bool WebAssembly::IsObjCNonFragileABIDefault() const { return true; }
+
+bool WebAssembly::UseObjCMixedDispatch() const { return true; }
+
+bool WebAssembly::isPICDefault() const { return false; }
+
+bool WebAssembly::isPIEDefault() const { return false; }
+
+bool WebAssembly::isPICDefaultForced() const { return false; }
+
+bool WebAssembly::IsIntegratedAssemblerDefault() const { return true; }
+
+// TODO: Support Objective C stuff.
+bool WebAssembly::SupportsObjCGC() const { return false; }
+
+bool WebAssembly::hasBlocksRuntime() const { return false; }
+
+// TODO: Support profiling.
+bool WebAssembly::SupportsProfiling() const { return false; }
+
----------------
No generic defaults here? Might also make sense to have these all inline if they're just return true/return false.

================
Comment at: lib/Driver/Tools.cpp:1564-1569
@@ +1563,8 @@
+
+#ifdef __wasm__
+    // Handle "native" by examining the host. "native" isn't meaningful when
+    // cross compiling, so only support this when the host is also WebAssembly.
+    if (CPU == "native")
+      return llvm::sys::getHostCPUName();
+#endif
+
----------------
I really dislike the idea of an ifdef here for this behavior. Can you explain some more? :)


Repository:
  rL LLVM

http://reviews.llvm.org/D12002