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

[Manman Ren via llvm-commits]

manmanren marked 14 inline comments as done.
manmanren added a comment.

Hi Quentin,

Thanks for reviewing the patch. I uploaded a new version to address your comments.

Cheers,
Manman


================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:1328
@@ +1327,3 @@
+// Return true if we should copy from swift error to the final vreg as specified
+// by SwiftErrorWorklist.
+static bool shouldCopySwiftErrorsToFinalVRegs(const TargetLowering &TLI,
----------------
For targets that support swifterror attribute, all accesses to swifterror values (with type swift_error_object**) will be converted to use the corresponding virtual register for the swifterror value. For targets that do not support swifterror attribute, the attribute will be ignored. We will load and store swift_error_object**.

================
Comment at: lib/CodeGen/SelectionDAG/FastISel.cpp:1361
@@ -1351,1 +1360,3 @@
+    if (shouldCopySwiftErrorsToFinalVRegs(TLI, FuncInfo))
+      return false;
     if (!handlePHINodesInSuccessorBlocks(I->getParent())) {
----------------
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.

================
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
----------------
+  /// 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].


http://reviews.llvm.org/D18108