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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D87371#2267053 <https://reviews.llvm.org/D87371#2267053>, @efriedma wrote:

>> the length of the synthesized canonical prologue might differ a little from the actual one maybe
>
> Getting the length wrong has caused miscompiles in the past.

I don't see how that would cause miscompiles here. But in any case, that experiment turned out to be less exciting than expected in the end; this patch, when properly working without being blocked by extra `.seh_handlerdata`, shaves off 2.5 KB from a xdata section of 228 KB, and such a relaxed matcher only got rid of another 2 KB. So for it to get any notable effect, the frame lowering needs to be adjusted anyway. But at least it has *some* effect now.



================
Comment at: llvm/lib/MC/MCWin64EH.cpp:933
 
+  if (PackedEpilogOffset == 0 && !info->HandlesExceptions &&
+      FuncLength <= 0x7ff && TryPacked) {
----------------
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.


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

https://reviews.llvm.org/D87371