[PATCH] D66013: [X86] Emit correct unwind info for split-stack prologue

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

I suspect there is some problem caused by interaction of .cfi_remember_state/.cfi_restore_state and CFIInstrInserter pass.

Considering the allocMBB is in the middle of the function. The CFA register is rbp instead of rsp in the prev MBB of allocMBB. At the beginning of allocMBB, CFIInstrInserter will insert a def_cfa_register to change the CFA register to rsp, which matches the state of function entry. The def_cfa_register will be inserted at the beginning of allocMBB and will be remembered by cfi_remember_state. Then .cfi_restore_state will restore the beginning of next MBB of allocMBB to have rsp instead of rbp as the CFA register, which is wrong.

If the cfi adjustment inserted by CFIInstrInserter pass is placed after .cfi_remember_state, it matches what we want here.



================
Comment at: lib/CodeGen/CFIInstrInserter.cpp:192-196
+        if (RememberedReg == ~0u) {
+          RememberedReg = SetRegister;
+          RememberedOffset = SetOffset;
+          break;
+        }
----------------
I feel it is not the correct meaning of .cfi_remember_state. .cfi_remember_state means remembering the current set of CFI rules so it is related with layout instead of function control flow. Here calculateOutgoingCFAInfo is based on function control flow. 

For allocMBB, the .cfi_remember_state remembers the state its prev MBB, not the state of its predecessor MBB, which is the function entry.

I think we should ignore .cfi_remember_state and .cfi_restore_state inside of calculateOutgoingCFAInfo because they are not directives associated with CFI changing machine instructions.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66013