This is an archive of the discontinued LLVM Phabricator instance.

[llvm-extract] Support extracting basic blocks
ClosedPublic

Authored by volkan on Dec 29 2017, 3:59 PM.

Details

Summary

Currently, there is no way to extract a basic block from a function easily. This patch
extends llvm-extract to extract the specified basic block(s).

Diff Detail

Event Timeline

volkan created this revision.Dec 29 2017, 3:59 PM
bogner added inline comments.Jan 9 2018, 4:31 PM
include/llvm/Transforms/IPO.h
181–189

The doc comment is either confusing or wrong here, I think. Could you please update it?

lib/Transforms/IPO/BlockExtractor.cpp
77–78

Should this use report_fatal_error or something instead of llvm_unreachable?

135–136

These should either be fatal errors (see above comment) or asserts. "if (x) unreachable" is a strange pattern.

137–139

llvm::find_if would save a few characters, if you prefer.

156

I guess CodeExtractor takes care of naming the extracted function, but could you double check that it does the right thing for name collisions? IE, if you extract bb1 from foo, but there's already a function called foo_bb1, what happens?

test/tools/llvm-extract/extract-block.ll
4

Worth checking that it got the right block? Could check for "%tmp5 = ", etc.

test/tools/llvm-extract/extract-invalid-block.ll
4

typo, s/contains/contain/ (pretty sure this test doesn't pass right now)

tools/bugpoint/ExtractFunction.cpp
410

Why this change?

tools/llvm-extract/llvm-extract.cpp
242–245

Not sure it's worth it, but I wonder if it would make sense to allow something like "-bb func1:bb2 -bb func2:bb3" to support extracting blocks from multiple functions.

volkan added inline comments.Jan 23 2018, 11:33 AM
lib/Transforms/IPO/BlockExtractor.cpp
156

Yes, it renames the function as foo_bb1.1 in that case.

tools/bugpoint/ExtractFunction.cpp
410

I think I renamed the option and reverted, but I forgot to remove this change. I will remove it.

volkan updated this revision to Diff 131120.Jan 23 2018, 11:36 AM

Updated based on the feedback.

volkan marked 7 inline comments as done.Jan 23 2018, 11:37 AM
bogner added inline comments.Jan 23 2018, 12:17 PM
tools/llvm-extract/llvm-extract.cpp
321–322

On interesting side effect of the "-bb function:block" approach is that we'll probably do something kind of strange if someone calls this like "llvm-extract -func f0 -bb f0:bb1" - will the result be functions f0 and f0_bb1, or will f0 be deleted and we only get f0_bb1? I suspect the latter. This is probably fine though - it seems unlikely that someone would really want to do that.

volkan added inline comments.Jan 23 2018, 12:24 PM
tools/llvm-extract/llvm-extract.cpp
321–322

Yes, it's going to be the latter. BlockExtractor erases the existing functions.

bogner accepted this revision.Jan 23 2018, 1:30 PM

Looks good. If people don't like the behaviour of mixing -func and -bb we can revisit later.

This revision is now accepted and ready to land.Jan 23 2018, 1:30 PM
This revision was automatically updated to reflect the committed changes.