This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Omit SEH directives for the epilogue if none are needed
ClosedPublic

Authored by mstorsjo on Oct 1 2020, 2:07 AM.

Details

Summary

For these cases, we already omit the prologue directives, if (!AFI->hasStackFrame() && !windowsRequiresStackProbe && !NumBytes).

When writing the epilogue (after the prolog has been written), if the function doesn't have the WinCFI flag set (i.e. if no prologue was generated), assume that no epilogue will be needed either, and don't emit any epilog start pseudo instruction. After completing the epilogue, make sure that we end up with matching epilog start/end.

Previously, when epilog start/end was generated, but no prologue, the unwind info for such functions actually was huge; 12 bytes xdata (4 bytes header, 4 bytes for one non-folded epilogue, 4 bytes for padded opcodes) and 8 bytes pdata. Because the epilog consisted of one opcode (end) but the prolog was empty (no .seh_endprologue), the epilogue couldn't be folded into the prologue, and thus couldn't be considered for packed form either.

On a 6.5 MB DLL with 110 KB pdata and 166 KB xdata, this gets rid of 38 KB pdata and 62 KB xdata.

Diff Detail

Event Timeline

mstorsjo created this revision.Oct 1 2020, 2:07 AM
mstorsjo requested review of this revision.Oct 1 2020, 2:07 AM
mstorsjo added inline comments.Oct 1 2020, 7:11 AM
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
1641

I guess an alternative, assert-less approach, would be not to set HaveWinCFI here, and remove the EpilogStart at the end, if it turns out not to be needed.

That explains what was happening in llvm/test/CodeGen/AArch64/lround-conv-fp16-win.ll etc.

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
1527

Not sure the HasEpilogStart boolean is useful here; could you just assert(HasWinCFI == MF.hasWinCFI())? And we shouldn't ever need to call setHasWinCFI().

1634

Simplify to if (MF.hasWinCFI()) {?

1734

Simplify this to if (HasWinCFI) {?

mstorsjo added inline comments.Oct 1 2020, 10:15 AM
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
1527

Right, that'd behave equivalently in the end - will try such a change.

mstorsjo updated this revision to Diff 295637.Oct 1 2020, 11:37 AM

Simplified the assert and conditionals.

This revision is now accepted and ready to land.Oct 1 2020, 12:23 PM