[PATCH] D12002: Initial WebAssembly support in clang

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 3 13:41:40 PDT 2015


echristo added inline comments.

================
Comment at: lib/Basic/Targets.cpp:7643-7649
@@ +7642,9 @@
+  case llvm::Triple::wasm64:
+    // Until specific variations are defined, don't permit any.
+    if (!(Triple == llvm::Triple("wasm64-unknown-unknown")) ||
+        (!Triple.getVendorName().empty() &&
+         Triple.getVendorName() != "unknown") ||
+        (!Triple.getOSName().empty() && Triple.getOSName() != "unknown") ||
+        (Triple.hasEnvironment() && Triple.getEnvironmentName() != "unknown"))
+      return nullptr;
+    return new WebAssemblyOSTargetInfo<WebAssembly64TargetInfo>(Triple);
----------------
sunfishcode wrote:
> echristo wrote:
> > Ditto.
> > 
> > (I said this just below, but it seems to have gotten munged in the newer version)
> I actually did see your comment and updated the code accordingly. It now does a positive test, `Triple == llvm::Triple("wasm64-unknown-unknown")`, which is simpler than what it did before.
> 
> However, it's also doing additional tests, because the Triple class's operator== doesn't distinguish between an Unknown that was actually "unknown" or an unknown that was some other string. Until we figure out what "vendor", "OS", and "environment" variations of wasm make sense (if any), we want to avoid dealing with accidental alternate triples.
I think this is a lot of overkill here, no other target cares about this so why should the wasm target? If it's that important maybe fix Triple?


Repository:
  rL LLVM

http://reviews.llvm.org/D12002





More information about the cfe-commits mailing list