[PATCH] D18046: [X86] Providing correct unwind info in function epilogue

[Kyle Butt via Phabricator via llvm-commits]

iteratee added a comment.

Some initial thoughts. I would like to hide the actual CFI algorithms from the existing passes as much as possible.



================
Comment at: lib/CodeGen/BranchFolding.cpp:412
+  // Outgoing cfa offset set by CurMBB after it's split.
+  int SetOffset = CurMBB.getIncomingCFAOffset();
+  // Outgoing cfa register set by CurMBB after it's split.
----------------
I'd like to see this factored out into MachineBasicBlock. 

Something like recalculateCFI(bool useExistingIncoming)


================
Comment at: lib/CodeGen/BranchFolding.cpp:1033
+
+      // Outgoing cfa offset set by BB.
+      int SetOffset = SameTails[i].getBlock()->getIncomingCFAOffset();
----------------
This looks like the same code above. Please factor this out.


================
Comment at: lib/CodeGen/TailDuplicator.cpp:596
     if (!MI.isPHI() && !MI.isDebugValue())
-      InstrCount += 1;
+      if (!MI.isCFIInstruction())
+        InstrCount += 1;
----------------
Can you create a function on MI: isDirective() and have it return true for debugvalue and CFIInstruction?


================
Comment at: lib/CodeGen/TailDuplicator.cpp:918
+
+    // Update the CFI info for PrevBB.
+    PrevBB->setOutgoingCFAOffset(TailBB->getOutgoingCFAOffset());
----------------
It would nice to not have this in the pass, but rather as an abstraction:

PrevBB->mergeCFIInfo(TailBB);


Repository:
  rL LLVM

https://reviews.llvm.org/D18046