Page MenuHomePhabricator

[BlockExtractor] Extend the file format to support the grouping of basic blocks
ClosedPublic

Authored by qcolombet on Apr 15 2019, 7:06 PM.

Details

Summary

Prior to this patch, each basic block listed in the extrack-blocks-file
would be extracted to a different function.

This patch adds the support for comma separated list of basic blocks
to form group.

When the region formed by a group is not extractable, e.g., not single
entry, all the blocks of that group are left untouched.

Let us see this new format in action (comments are not part of the
file format):
;; funcName bbName[,bbName...]
foo bb1 ;; Extract bb1 in its own function
foo bb2,bb3 ;; Extract bb2,bb3 in their own function
bar bb1,bb4 ;; Extract bb1,bb4 in their own function
bar bb2 ;; Extract bb2 in its own function

Assuming all regions are extractable, this will create one function and
thus one call per region.

Diff Detail

Repository
rL LLVM

Event Timeline

qcolombet created this revision.Apr 15 2019, 7:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2019, 7:06 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

Hi Quentin,

This looks good to me, but seems like a bit complicated. I think we can simplify the logic by getting basic block groups as a comma separated string.
For example:

foo bb1 ;; Extract bb1 in its own function
foo bb2,bb3 ;; Extract bb2 and bb3 in their own function

What do you think?

Hi Volkan,

Good point!

I ended up tailoring the syntax to my current use case (which basically adds group ids at the end of the exiting file with sed), but your syntax is much easier to understand!

Updating!
Cheers,
-Quentin

Update:

  • Use comma separated list instead of group naming.
volkan accepted this revision.Apr 18 2019, 10:14 AM

LGTM, thanks Quentin. Could you please update the summary before pushing this?

This revision is now accepted and ready to land.Apr 18 2019, 10:14 AM
qcolombet edited the summary of this revision. (Show Details)Apr 18 2019, 10:31 AM

Could you please update the summary before pushing this?

Done!

Thanks!!

This revision was automatically updated to reflect the committed changes.