This is an archive of the discontinued LLVM Phabricator instance.

[WinEH] Iterate state changes instead of invokes
ClosedPublic

Authored by JosephTremoulet on Oct 10 2015, 12:13 PM.

Details

Summary

Add an iterator that can walk across blocks and which visits the state
transitions rather than state ranges, with explicit transitions to -1
indicating the presence of top-level calls that may throw and cause the
current function to unwind to caller. This will simplify code that needs
to identify nested try regions.

Refactor SEH and C++EH table generation to use the new
InvokeStateChangeIterator, and remove the InvokeLabelIterator they were
using.

Diff Detail

Event Timeline

JosephTremoulet retitled this revision from to [WinEH] Iterate state changes instead of invokes.
JosephTremoulet updated this object.
JosephTremoulet added a subscriber: llvm-commits.

If there's something I should do to test this other than 'ninja check' and 'ninja clang-test', please point me at it.

Use std::next/prev

rnk accepted this revision.Oct 12 2015, 2:14 PM
rnk edited edge metadata.

lgtm

Thanks for taking this on, I think the state machine for detecting invoke ranges is pretty subtle and I'd rather not have three of them. It's not as simple as I hoped it'd be, but it seems like an improvement over what we had before.

lib/CodeGen/AsmPrinter/WinException.cpp
408

You can save a level of indentation by handling the MI.isCall() case before the isEHLabel() check, and then changing this check to

if (!MI.isEHLabel())
  continue;
536

We don't actually need this std::next, the entry block should never return true for isEHFuncletEntry. David pointed this out to me in person last week.

This revision is now accepted and ready to land.Oct 12 2015, 2:14 PM

Thanks for the review!

lib/CodeGen/AsmPrinter/WinException.cpp
408

Done, thanks.

536

I'm inclined to leave this one as is:

  • I feel like it's a bit more readable this way
  • While removing && MBB != MF->begin() in the previous version would have saved some work (a useless test in each iteration of the loop), keeping the call to std::next above the loop in this version actually saves a tiny bit of work (one unnecessary iteration of the loop)

Happy to come back and change it if you feel strongly, though.

JosephTremoulet edited edge metadata.