[PATCH] D125645: [ARM SEH 3] [ARM] [MC] Add support for writing ARM WinEH unwind info

Mon May 16 13:41:14 PDT 2022

efriedma added a comment.

In D125645#3516998 <https://reviews.llvm.org/D125645#3516998>, @mstorsjo wrote:

> In D125645#3516878 <https://reviews.llvm.org/D125645#3516878>, @efriedma wrote:
>
>> It looks like this doesn't include any directive to set the "F" bit in xdata,
>
> That's true. If we'd have unwind info splitting implemented, that bit would be set on the follow-up fragments, implicitly. Maybe there's a usecase for manually creating such unwind data though? I think that one can be added without much troubles later, if there's a concrete demand for it though...

We would want this to split a function across multiple section (e.g. for hot/cold splitting).

> (On that note, when rereading the docs about the "F" bit, I'm left wondering how that's supposed to work. If we have a huge function, and describe a prologue-less fragment in the middle of the function - how will unwinding from it work, when there's no prologue opcodes in that fragment to use for unwinding, if unwinding starts from outside of any potential epilogue in it, as there also can be fragments without epilogues?)

I'm pretty sure the idea is that you still emit the prologue opcodes from the original prologue.  The "F" bit just means that the prologue is not part of the current fragment.  (For AArch64, this idea was generalized with the "c_end" opcode.)

>> or the "Condition" field in epilogue scopes.  Is the omission intentional?  (I assume your LLVM patch won't emit sequences that would require either of those, but that might change, and people writing assembly might want them.)
>
> Right, that's probably a bit bigger omission. Yeah it's not needed by our generated code, but it would probably be good to be able to express it if we wanted to. Maybe `.seh_startepilogue_cond <cond>` or something like that? (We'd probably want to have the default `.seh_startepilogue` as is - I think.)

Sure.

================
Comment at: llvm/include/llvm/Support/Win64EH.h:79
+  UOP_WideSaveRegMask,
+  // Using UOP_SetFP from above
+  UOP_SaveRegsR4R7LR,
----------------
mstorsjo wrote:
> efriedma wrote:
> > Not sure I like calling this operation UOP_SetFP, given it can set an arbitrary register.
> Oh, right, I guess I brought over the design a bit too much from the aarch64 case here, while it's notably different here.
> 
> Should we call it `UOP_SaveSP` and `.seh_save_sp` then?
That sounds reasonable.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125645