[PATCH] D114547: [AArch64] Async unwind - Adjust unwind info in AArch64LoadStoreOptimizer

Thu Dec 9 02:10:23 PST 2021

chill added a comment.

In D114547#3152164 <https://reviews.llvm.org/D114547#3152164>, @efriedma wrote:

> This could use a big comment explaining that AArch64LoadStoreOpt can run after PEI, and what that means in terms of the invariants we rely on, and the invariants we need to uphold to preserve unwind info.  I can see what this patch is doing, but it's not obvious this is a comprehensive solution.  Are sp adjustments the only CFI info we need to update?

It's entirely possible this is not a comprehensive solution and I certainly wouldn't claim it was. I just have not noticed or been able to think of any other cases.

One possible thing is merging an `SP` update to a //preceding// load/store - if it jumps over a few unrelated instructions the CFA offset would be incorrect for these few instructions.
Such merging is done during epilogue emission, so CFI instructions are emitted accordingly and I couldn't steer the compiler into leaving that to load/store optimiser, where it
wold break the unwind info (and by "break" I mean a small degradation in the user experience when debugging or a slight decline in the precision of sampling profilers, certainly
nothing the affects code correctness).

> Does it matter if the load/store we're modifying already has a CFI directive attached to it?

I shouldn't. It would at most create some redundancy if the load/store already wrote-back to `SP` and had a `.cfi_def_cfa_offset`, or `.cfi_def_cfa` immediately following it.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114547/new/

https://reviews.llvm.org/D114547