Page MenuHomePhabricator

[AArch64] [Windows] Share unwind codes between epilogues with identical unwind codes
ClosedPublic

Authored by ssijaric on Jan 16 2019, 3:00 PM.

Details

Summary

There are cases where we have multiple epilogues that have the exact same unwind code sequence. This change merges such epilogues into a single epilogue.

This should get us past the assert "SEH unwind data splitting not yet implemented" in many cases. We still need to add support for generating multiple .pdata/.xdata sections for those functions that need to be split into fragments.

Diff Detail

Repository
rL LLVM

Event Timeline

ssijaric created this revision.Jan 16 2019, 3:00 PM
dmajor added a subscriber: dmajor.Jan 16 2019, 3:09 PM
TomTan added a subscriber: TomTan.Jan 16 2019, 3:12 PM

It might be nice to merge together identical suffixes eventually, but that shouldn't block this change.

How hard would it be to add support for reusing opcodes from an identical prolog? The unwind format is designed to allow that in some cases.

lib/MC/MCWin64EH.cpp
459 ↗(On Diff #182156)

Using a pair here seems weird... either use llvm:Optional, or just return a null pointer.

468 ↗(On Diff #182156)

A bool would be sufficient here.

513 ↗(On Diff #182156)

The DenseMaps here seem unnecessary; I mean, yes, it'll reduce the number of potential matching epilogs, but realistically functions have very few unique epilogs anyway.

529 ↗(On Diff #182156)

It's not immediately obvious that you're mutating the EpilogMap here; probably worth calling that out explicitly in a comment. I guess it does the right thing because nothing else will use the FrameInfo after this.

I've confirmed that I can build Firefox successfully with this patch on top of latest trunk. Thank you @ssijaric!

ssijaric updated this revision to Diff 182187.Jan 16 2019, 4:34 PM

Address Eli's comments.

This revision is now accepted and ready to land.Jan 16 2019, 4:44 PM

I've confirmed that I can build Firefox successfully with this patch on top of latest trunk. Thank you @ssijaric!

Hi David,

Can you please try with the latest version of the patch? I am going to commit it shortly.

Thanks.

I've confirmed that I can build Firefox successfully with this patch on top of latest trunk. Thank you @ssijaric!

Hi David,

Can you please try with the latest version of the patch? I am going to commit it shortly.

Thanks.

FYI, confirmed that V8 builds successfully with the latest change which had the issue. But I am not sure about Firefox.

The updated patch works for Firefox too.

This revision was automatically updated to reflect the committed changes.