[PATCH] D18108: Swift Calling Convention: SelectionDAGBuilder changes to support swifterror

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:1361
@@ -1351,1 +1360,3 @@
+    if (shouldCopySwiftErrorsToFinalVRegs(TLI, FuncInfo))
+      return false;
     if (!handlePHINodesInSuccessorBlocks(I->getParent())) {
----------------
manmanren wrote:
> If we need to materialize the vreg from worklist (i.e shouldCopySwiftErrorsToFinalVRegs returning true), we bail out of FastISel, so we should not need to call handlePHINodesInSuccessorBlocks?
> 
> Oops, forgot to update the comment.
Ah make sense. Are you planning to update the comment?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5724
@@ +5723,3 @@
+      HasSwiftError = true;
+      // A function can only have a single swifterror argument. And if it does
+      // have a swifterror argument, it must be the first entry in
----------------
manmanren wrote:
> +  /// A function can only have a single swifterror argument. And if it does
> +  /// have a swifterror argument, it must be the first entry in
> +  /// SwiftErrorVals.
> +  SwiftErrorValues SwiftErrorVals;
> 
> When setting up SwiftErrorVals, we always put the argument as the first entry, so findSwiftErrorVReg should return SwiftErrorMap[FuncInfo.MBB][0].
Couldn’t we feed the argument with another swift error?
E.g.,
define void @foo(swift error %a)
  %b = alloca swift error
  call void @bar(%b) ; <— Here b is not in the first value in swifterrorvals, does it?


http://reviews.llvm.org/D18108