[PATCH] D125433: [ARM64][SEH] PR54879: Packed Unwind Info when Homing Int Param Regs

Thu May 12 04:28:32 PDT 2022

mstorsjo added inline comments.

================
Comment at: llvm/lib/MC/MCWin64EH.cpp:640
+      ENops++; EI--; continue;
+    }
+    // If we reach here, we find unmatching ops that are NOT UOP_Nop.
----------------
efriedma wrote:
> We also use checkPackedEpilog() for to compute the epilogue offset in xdata records; we can't mess with the result for the sake of analyzing packed unwind.  If you need a different analysis, please split it into a separate function, or use a flag.  (Probably easier to understand if you split it into a separate function?)
+1, indeed. If we should tolerate a differing epilogue for the case of packed unwind info, we need a separate function to determine if an epilogue matches a prologue, for a specific packed form. Otherwise, we'd end up trying to share epilogues with prologues (for the non-packed format) where they actually don't match.

However, I'd like to take this whole issue back to the drawing board first; let me follow up on the bug report, to first exactly nail the expected behaviour.

================
Comment at: llvm/test/MC/AArch64/seh-packed-unwind.s:509
     nop
     .seh_nop
     ldr x19, [sp], #80
----------------
If we would proceed with this patch, then we must also change the behaviour for the existing testcase for `func5`. For a specific packed form of unwind data, there can't be any ambiguity whether an epilog contains nops or not - we can't allow both cases. Let me follow up on the original bug report on that topic.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125433