[PATCH] D50423: [WebAssembly] Gate i64x2 and f64x2 on -wasm-enable-unimplemented

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:49
+    cl::desc("Emit potentially-unimplemented WebAssembly instructions"),
+    cl::init(false));
+
----------------
tlively wrote:
> aheejin wrote:
> > Are we planning to use this flag only for simd or also for other instructions? For example, exception handling is not currently implemented in the VM yet, but we are adding opcodes for it (under `-mattr=+exception-handling` flag). And in case if other stage 0~1 proposals come up, we may want to add them in the toolchain before they are implemented in the VM. Are we gonna use this flag for all these cases? If not, how about changing it to `wasm-enable-unimplemented-simd`?
> It may make sense to use the same flag for all cases where ops are implemented in the toolchain but not the VM. However, I'd be happy to change the name if this will only be used for SIMD. Would you want to have a single general flag for this use case, or are other existing mechanisms sufficient?
I don't think we need a separate flag that handles all unimplemented opcodes because we already have [[ https://github.com/llvm-mirror/llvm/blob/aae8303b43627148c6d80de0d9125bb168b5e155/lib/Target/WebAssembly/WebAssembly.td#L26-L42 | feature flags ]] for each not-fully-implemented-and-ongoing feature. If we have enough cases like this, where a feature is controlled by a feature flag but we need a separation even within it, using this flag for all those cases might make sense. But before that, I guess making it clear that this applies only for SIMD is less confusing, because currently it sounds like it encompasses all unimplemented features.


Repository:
  rL LLVM

https://reviews.llvm.org/D50423