Page MenuHomePhabricator

[llvm-mca] Add the ability to mark regions of code for analysis (PR36875)

Authored by andreadb on Apr 9 2018, 3:58 AM.



This patch teaches llvm-mca how to parse code comments in search for special "markers" used to select regions of code.


# LLVM-MCA-START My Code Region

The MCAsmLexer now delegates to an object of class MCACommentParser (i.e. an AsmCommentConsumer) the parsing of code comments to search for begin/end code region markers.

A comment starting with substring "LLVM-MCA-START" marks the beginning of a new region of code.
A comment starting with substring "LLVM-MCA-END" marks the end of the last region.

This implementation doesn't allow regions to overlap. Each region can have a optional description; internally, each region is described by a range of source code locations (SMLoc).

MCInst objects are added to a region R only if the source location for the MCInst is in the range of locations specified by R.

By default, the tool allocates an implicit "Default" code region which contains every source location.
See new tests llvm-mca-marker-*.s for a few examples.

How this affect the report generated by the tool?

A new Backend object is created for every region. So, the analysis is conducted on every parsed code region.
The final report is the union of the reports generated for every code region. Note that empty regions are skipped.

Special "[#] Code Region - ..." strings are used in the report to mark the portion which is specific to a code region only. For example, see llvm-mca-markers-5.s.

Please let me know if okay to commit.


Diff Detail


Event Timeline

andreadb created this revision.Apr 9 2018, 3:58 AM
andreadb edited the summary of this revision. (Show Details)Apr 9 2018, 4:01 AM
courbet accepted this revision.Apr 9 2018, 5:16 AM

Only minor comments, LGTM otherwise.

27 ↗(On Diff #141609)

assert(Regions.empty() && "missing Default region");
(for the reader)

41 ↗(On Diff #141609)


9 ↗(On Diff #141609)

Please add a file comment.

27 ↗(On Diff #141609)

I think having the const outside of the type would make semantics of InstVec& Sequence and InstVec &MCInstSequence clearer.

This revision is now accepted and ready to land.Apr 9 2018, 5:16 AM
andreadb added inline comments.Apr 9 2018, 5:49 AM
9 ↗(On Diff #141609)

Ouch... I thought that something was missing.

Thanks for spotting it. I will add it.

RKSimon accepted this revision.Apr 9 2018, 8:30 AM

LGTM once the docs/comments have been updated

andreadb marked 4 inline comments as done.Apr 9 2018, 9:33 AM
This revision was automatically updated to reflect the committed changes.
RKSimon added a subscriber: MaskRay.Apr 9 2018, 2:55 PM

This should be fixed by rL329592 - thanks @MaskRay