[PATCH] D76049: [WebAssembly] Support swiftself and swifterror for WebAssembly target

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp:247
+  // if there aren't. Forward the cursor for the extra parameters.
+  if (MF.getFunction().getCallingConv() == CallingConv::Swift) {
+    bool HasSwiftErrorArg = false;
----------------
Is this bit actually necessary? in your change to `WebAssemblyTargetLowering::LowerFormalArguments` you do `MFI->addParam()` for the extra prams, so this should already be reflected in the value of `CurLocal` at this point? 


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:815
+      CLI.Outs.push_back(Arg);
+      SDValue ArgVal = DAG.getUNDEF(PtrVT);
+      CLI.OutVals.push_back(ArgVal);
----------------
dschuff wrote:
> I assume this will actually be hooked up to some value instead of being undef? Where should these values actually come from? Are all functions expected to have these params?
OK, I think i understand now. This is for when the callee takes swiftself/swifterror but the caller is not going to pass real values. What if the callee actually needs the self parameter though?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.h:163
 // signature for F (F is just used to get varous context)
-void computeSignatureVTs(const FunctionType *Ty, const Function &F,
-                         const TargetMachine &TM, SmallVectorImpl<MVT> &Params,
+void computeSignatureVTs(const FunctionType *Ty, const Function *TargetFunc,
+                         const Function &ContextFunc, const TargetMachine &TM,
----------------
Since we are now passing a pointer to `TargetFunc` directly, can we avoid also passing `Ty` (and just get the FunctionType inside computeSignatureVTs)?
Please also update the comment.


================
Comment at: llvm/test/CodeGen/WebAssembly/swiftcc.ll:17
+; CHECK: call_indirect   (i32, i32, i32, i32) -> ()
+define swiftcc void @bar() {
+  %1 = load i8*, i8** @data
----------------
There should also be tests for casting in both directions (e.g. from a function with declared swiftself to one without and vice versa), and at least one where swifterror is used instead of swiftself. (e.g. try to cover all the new cases you added).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76049