sunfishcode 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);
----------------
echristo wrote:
> 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?
This Triple issue is not important enough to hold up the rest of the patch for,
so I just removed it. Thanks for the review!
Repository:
rL LLVM
http://reviews.llvm.org/D12002
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
