[PATCH] D87371: [MC] [Win64EH] Try to generate packed unwind info where possible

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:933
 
+  if (PackedEpilogOffset == 0 && !info->HandlesExceptions &&
+      FuncLength <= 0x7ff && TryPacked) {
----------------
mstorsjo wrote:
> efriedma wrote:
> > mstorsjo wrote:
> > > efriedma wrote:
> > > > Is it always exactly zero?  Spec says in some cases "No instruction corresponding to mov x29,sp is present in the epilog"
> > > I'm not entirely sure how to interpret that clause.
> > > 
> > > I'm looking at one case, where MSVC generates the following prologue:
> > > ```
> > >        0: fd 7b bb a9   stp     x29, x30, [sp, #-80]!
> > >        4: fd 03 00 91   mov     x29, sp
> > >        8: ff 43 00 d1   sub     sp, sp, #16
> > > ```
> > > However, the final `sub sp, sp, #16` isn't part of the canonical prologue, and this is described as packed unwind info RegI=0, CR=3, FrameSize=80, which llvm-readobj prints as
> > > ```
> > >     Prologue [
> > >       mov x29, sp
> > >       stp x29, lr, [sp, #-80]!
> > >       end
> > >     ]
> > > ```
> > > However the actual epilogue of the generated code looks like this:
> > > ```
> > >       dc: ff 43 00 91   add     sp, sp, #16
> > >       e0: fd 7b c5 a8   ldp     x29, x30, [sp], #80
> > > ```
> > > 
> > > (Here, instead of `mov sp, x29` it has `add sp, sp #16`, but it doesn't matter as they're equivalent here.)
> > > 
> > > So if unwinding from the body, the prologue is executed, and it starts off by reversing `mov x29, sp` from the prologue. But if choosing to execute the epilogue instead of the prologue, it has to execute that instruction, to get the right stack pointer.
> > > 
> > > From testing the unwinder behaviour with RtlVirtualUnwind, it does look like it does execute `mov sp, x29` as part of the epilogue for these cases though, which would make sense.
> > > 
> > > 
> > > So I guess it should be ok to generate packed data if the epilogue is an exact match of the prologue, except for the final `set_fp` opcode - but that wording actually makes me worry more about the cases where the epilogue does contain that opcode (and rely on it). And the docs say
> > > 
> > > > Packed unwind data can't be used if a function requires restoration of sp from x29.
> > > 
> > > But I'd say that the code generated by MSVC that I quoted does seem to rely on it.
> > Makes sense.
> > 
> > Maybe we can get a spec clarification for this.
> I'll see if I can get @TomTan to comment on it.
I discussed this with @TomTan, and he clarified that the synthesized epilogue for these cases lacks the corresponding `set_fp` opcode - because makes it suitable in a lot more cases for the kind of code MSVC often generates.

But he also agreed that the wording in the docs seems to be overly strict and the statement "Packed unwind data can't be used if a function requires restoration of sp from x29." doesn't seem to hold up (and agrees with the reasoning above that it would unwind correctly anyway), and he checked with the original document author who can't remember a reason why it wouldn't work either. So he agrees we can proceed with this patch, generating the packed format even if the epilogue is an exact mirror of the prologue, including the set_fp opcode.



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

https://reviews.llvm.org/D87371