[PATCH] D86529: [5/5] [AArch64] Generate and parse SEH assembly directives

Wed Aug 26 14:29:34 PDT 2020

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM

================
Comment at: llvm/test/CodeGen/AArch64/lrint-conv-fp16-win.ll:7
+; CHECK-NEXT:  .seh_startepilogue
+; CHECK-NEXT:  .seh_endepilogue
 ; CHECK-NEXT:  ret
----------------
mstorsjo wrote:
> efriedma wrote:
> > mstorsjo wrote:
> > > efriedma wrote:
> > > > I think this indicates a bug somewhere else: we shouldn't be emitting seh_startepilogue in functions that don't have any other unwind info.
> > > No, we do generate full proper directives for start/endproc, prologue and all that. It's just that the current testcase doesn't break if there's changes in the prologue of the function, but the sequence of `CHECK-NEXT` from `fcvtzs` to `ret` requires including these directives here in order not to break the test.
> > > 
> > > So for these testcases, that focus on testing some other aspect, I did the minimal changes to include the new directives only where they're needed in `CHECK-NEXT` sequences, but didn't go full out and added checks for all the generated directives. But I'll include one such case as well.
> > > 
> > > Alternatively, one could add `nounwind` attributes to the tested functions, to avoid the noise from the added directives. (Is it possible to achieve the same with some llc flag? I tried looking for one but didn't find any.)
> > Should we be generating any .seh_* directives in this case?  If I'm not mistaken, we don't generate any actual unwind data in the object file.
> The llc command in this testcase does generate unwind data in the generated object file, if it's written to an object file instead of output as asm.
Hmm, I see, we emit a trivial xdata with just an "end" opcode.  We probably shouldn't, but I guess you don't need to address that in this patch.

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

https://reviews.llvm.org/D86529