[PATCH] D114545: [CodeGen] Async unwind - add a pass to fix CFI information

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D114545#3414572 <https://reviews.llvm.org/D114545#3414572>, @chill wrote:

> In D114545#3409903 <https://reviews.llvm.org/D114545#3409903>, @MaskRay wrote:
>
>> "layout order" seems to suggest the linear placement order.
>
> That's right, it's the order of increasing instruction addresses, corresponding to the basic block layout and the order of instructions in the basic block.

OK

>> In a function using SP as the CFA, if a call needs to temporarily adjust the stack size (e.g. x86 pushl/pushq; in x86-32 pushl is particular common
>> because of the calling convention), llvm may produce something like `.cfi_adjust_cfa_offset {4,8}`. Can AArch64 codegen produce something like this?
>
> I don't think AArch64 ever generates `.cfi_adjust_cfa_offset`.

OK. Agreed according to the call sites of `emitCFIAdjustCfaOffset`.

>> Whose responsibility is to emit a negative `.cfi_adjust_cfa_offset` to cancel the effect? CFIFixup cannot handle such a case while CFIInstrInserter can handle it.
>
> Indeed, if a backend uses SP as a frame pointer and allocates/deallocates outgoing arguments space in different basic blocks, CFIFixup won't fix it.

Agree.

>> See `test/CodeGen/X86/segmented-stacks-dynamic.ll`: rbx may be used as a base register and CFIInstrInserter can insert `.cfi_restore %rbx`. CFIFixup doesn't support this.
>
> As far as I can understand this testcase, the blocks containing `.cfi_restore %rbx` would have `StrongNoFrameOnEntry` set, which would make CFIFixup
> fall into the "reset to initial state".
>
> FWIW, for the epilogue blocks the test, the PEI should have already inserted `.cfi_restore` for the pops of the CSRs.

Perhaps `StrongNoFrameOnEntry` has not clearly been explained:) hence my confusion.


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

https://reviews.llvm.org/D114545