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

[Kyle Butt via Phabricator via llvm-commits]

iteratee added a comment.

This is coming along nicely. I forgot to say last time that I was pleased overall.

I have a few more things, but they're slowly getting smaller.



================
Comment at: lib/CodeGen/BranchFolding.cpp:329
     --I2;
     while (I2->isDebugValue()) {
       if (I2 == MBB2->begin())
----------------
should this be isDirective?


================
Comment at: lib/CodeGen/BranchFolding.cpp:346
+
+  // Cover the case of having I1 or I2 point to a CFI_INSTRUCTION. This can
+  // happen if I1 and I2 are non-identical when compared and then one or both of
----------------
Is this block still necessary if the tests above are changed to isDirective? I'm having trouble making sense of them


================
Comment at: lib/CodeGen/MachineBasicBlock.cpp:1443
+    case MCCFIInstruction::OpDefCfaOffset:
+      setOutgoingCFAOffset(-CFI.getOffset() + AdjustAmount);
+      setAdjustCFAOffset(AdjustAmount);
----------------
Why is this offset negative? Can you explain it to me?


Repository:
  rL LLVM

https://reviews.llvm.org/D18046