[PATCH] D121349: [WebAssembly] Second phase of implementing extended const proposal.

[Derek Schuff via Phabricator via llvm-commits]

dschuff added a comment.

I assume these `llvm_unreachables` your're adding in this CL are the places that will be filled in in subsequent CLs?



================
Comment at: llvm/include/llvm/ObjectYAML/WasmYAML.h:67
+  bool Extended;
+  wasm::WasmInitExprMVP Inst;
+  yaml::BinaryRef Body;
----------------
Should these be a union?
I wonder when we can go to C++17 variant...


================
Comment at: llvm/lib/MC/WasmObjectWriter.cpp:935
+    if (Global.InitExpr.Extended) {
+      llvm_unreachable("extected init expressions not supported");
+    } else {
----------------



================
Comment at: llvm/lib/Object/WasmObjectFile.cpp:200
+  if (!Expr.Extended) {
+    uint8_t EndOpcode = readOpcode(Ctx);
+    if (EndOpcode != wasm::WASM_OPCODE_END)
----------------
this is a bit confusing. If there's just a const/global.get/ref.null followed by an end, it's MVP, otherwise it's extended. But can there be 2 consts in a row? Seems like that would also be accepted here? and then line 208 would read a 3rd opcode?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121349/new/

https://reviews.llvm.org/D121349