aheejin accepted this revision.
aheejin added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:741
+ explicit WebAssemblyABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind _Kind)
+ : SwiftABIInfo(CGT), defaultInfo(CGT), Kind(_Kind) {}
----------------
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.
================
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)
----------------
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?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72972/new/
https://reviews.llvm.org/D72972
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
