This is an archive of the discontinued LLVM Phabricator instance.

[CodeExtractor] Allow extracting blocks with exception handling
ClosedPublic

Authored by sdmitriev on Apr 20 2018, 2:03 PM.

Details

Summary

This is a CodeExtractor improvement which adds support for extracting blocks which have exception handling constructs if that is legal to do. CodeExtractor performs validation checks to ensure that extraction is legal when it finds invoke instructions or EH pads (landingpad, catchswitch, or cleanuppad) in blocks to be extracted.

I have also added an option to allow extraction of blocks with alloca instructions, but no validation is done for allocas. CodeExtractor caller has to validate it himself before allowing alloca instructions to be extracted. By default allocas are still not allowed in extraction blocks.

Diff Detail

Event Timeline

sdmitriev created this revision.Apr 20 2018, 2:03 PM
rnk added inline comments.Apr 25 2018, 9:48 AM
lib/Transforms/Utils/CodeExtractor.cpp
207–209

Do not attempt to recover from broken invariants. There are three reasonable options:

  1. If this condition is possible but undesirable under normal operation, log with LLVM_DEBUG, dbgs(), and return {}.
  2. Assert that this condition is impossible.
  3. Use report_fatal_error to assert this condition is possible, but don't compile it out from release builds.
217–220

Ditto, this should probably be an assert, with the whole loop inside an NDEBUG block like it was before.

test/Transforms/CodeExtractor/inline_eh.ll
2

Please add a similar test that uses _CxxFrameHandler3 to get coverage of all the code you added to handle cleanupret, catchret, etc.

sdmitriev added inline comments.Apr 27 2018, 9:45 AM
lib/Transforms/Utils/CodeExtractor.cpp
207–209

Ok, I will log a message with dbgs(), that will preserve existing behavior. buildExtractionBlockSet was returning an empty list when input list contained a block that was not valid to extract.

217–220

This is actually an important part of the validation check which verifies than extraction is legal. So moving it under NDEBUG is undesirable since this code won’t run in release build. I will log a message and return an empty list.

test/Transforms/CodeExtractor/inline_eh.ll
2

Sure. I will add such test.

sdmitriev updated this revision to Diff 144364.Apr 27 2018, 9:48 AM
rnk accepted this revision.May 10 2018, 3:43 PM

lgtm

This revision is now accepted and ready to land.May 10 2018, 3:43 PM
This revision was automatically updated to reflect the committed changes.