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

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added a comment.

Hi,

I see that your patch is mostly about the state of CFA, but I think this applies to other things, like callee-saved registers.

As part of my work on shrink-wrapping, I needed to add support for saving / restoring registers in multiple locations. So, in order to get correct CFI information for the registers, we have to add the corresponding `.cfi_offset` and `.cfi_restore` to the save and restore points, which results in the following code:

  // BLOCK0
  if (i) {
      // BLOCK1
      save reg1
      if (a) {
        // BLOCK2
        save reg2
        [... use reg1 reg2 ...]
        restore reg2
        restore reg1
        ret
      } else {
        // BLOCK3
        save reg3
        [... use only reg1, reg3 ...]
        restore reg3
        restore reg1
        ret
      }
  } else {
   // BLOCK4
   ret
  }

So, in this case, it's not correct to assume that the CFI state starts at the entry block, nor to assume that it is the same in the whole function (pretty much the same issue as CFA). For this, I emit a `CFI_INSTRUCTION .cfi_offset` along with every save, and a `CFI_INSTRUCTION .cfi_restore` along with every restore. After that, in the AsmPrinter I run another pass to fill the gaps and add more `.cfi_offset` or `.cfi_restore` where the fall through state is not correct.

Imagine the order of the basic blocks was:

  BLOCK0
  BLOCK1
  BLOCK4
  BLOCK2
  BLOCK3

then we need an extra `.cfi_restore reg1` at the end of `BLOCK1` and an extra `.cfi_offset reg1` at the beginning of `BLOCK2`.

This could currently happen with the current shrink-wrapping implementation, but the block placement is never placing blocks in a way that the prologue comes after the epilogue, so we didn't hit that issue yet.

I guess this issue can be also addressed with the same infrastructure you're building here for CFA. What do you think?

Also, I might be missing something here, but do we really need to maintain the state of CFA in every MBB starting from PEI to (almost) AsmPrinter? Can't we just do one simple pass during (or before) AsmPrinter which collects all the CFI directives and does the analysis based on the CFG and the final block layout (since at that point, we have that information) ? It looks like TailDuplication was the problem, but looks like you fixed that to skip CFI instructions.

Let me know what you think.


Repository:
  rL LLVM

https://reviews.llvm.org/D18046