[PATCH] D72972: [WebAssembly] Add experimental multivalue calling ABI

[Thomas Lively via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:741
+  explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind _Kind)
+      : SwiftABIInfo(CGT), defaultInfo(CGT), Kind(_Kind) {}
 
----------------
aheejin wrote:
> tlively wrote:
> > aheejin wrote:
> > > Nit: Do we need `_`?
> > I was cargo culting the ARM targets here. It may be that some compilers get confused when the type and variable have the same name in this situation?
> I don't think so. We have many cases that use the same name for a parameter and its corresponding field in constructors.
Ok, dropped it!


================
Comment at: clang/test/CodeGen/wasm-arguments.c:19
+
 // Structs should be passed byval and not split up.
+// WEBASSEMBLY32: define void @struct_arg(%struct.s1* byval(%struct.s1) align 4 %i)
----------------
aheejin wrote:
> tlively wrote:
> > aheejin wrote:
> > > The comment here includes only the MVP case. Shouldn't we update this? Ditto for the other comments below. 
> > I think there is some value in having the comments describe the expected stable behavior and let the "EXPERIMENTAL" speak for itself. The only thing I can think of adding to the comment is "... in non-experimental ABIs", but that doesn't seem too valuable to have.
> Then we can maybe add one-line comment for `EXPERIMENTAL-MV`s too in case its results deviates from MVP?
Sounds good 👍


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72972