[PATCH] D69156: [WebAssembly] Allow multivalue signatures in object files

[Thomas Lively via Phabricator via llvm-commits]

tlively marked 2 inline comments as done.
tlively added inline comments.


================
Comment at: llvm/lib/Object/WasmObjectFile.cpp:887
-        return make_error<GenericBinaryError>(
-            "Multiple return types not supported", object_error::parse_failed);
-      }
----------------
sbc100 wrote:
> Do we want to error out somehow if the feature flags of the object file don't include multi-value?
That's an interesting idea! I don't think we have a precedent for that kind of validation in the object file parser. It would also be somewhat intrusive, since the target features section is one of the last sections to be parsed, so there would have to be a separate step at the end where feature invariants are checked. The invariants we could check in such a step would also be limited since individual instructions are not exposed in the object file abstraction.


================
Comment at: llvm/test/CodeGen/WebAssembly/multivalue.ll:11
 %pair = type { i32, i32 }
 %packed_pair = type <{ i32, i32 }>
 
----------------
aheejin wrote:
> Unrelated question to this CL: Do we treat pair and packed pair in the same way?
Hmm, I guess this isn't a great test because there wouldn't be any padding in this struct anyway, so in this case it literally is the same. I'm not sure what would happen with a packed struct that should contain padding. I'll add a better test in a follow up CL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69156