<div dir="ltr"><div>Hi Matt,</div><div><br></div><div>could you please update the associated patch too?</div><div><br></div><div>Thanks</div><div>-Andrea<br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 17, 2018 at 5:48 PM via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Adding the llvm-dev list, because my email client decides to remove certain lists when I reply-all... including the list that I intend to respond to.<br>
<br>
> -----Original Message-----<br>
> From: Davis, Matthew <<a href="mailto:Matthew.Davis@sony.com" target="_blank">Matthew.Davis@sony.com</a>><br>
> Sent: Monday, December 17, 2018 9:47 AM<br>
> To: Davis, Matthew <<a href="mailto:Matthew.Davis@sony.com" target="_blank">Matthew.Davis@sony.com</a>>; <a href="mailto:llvm-dev@redking.me.uk" target="_blank">llvm-dev@redking.me.uk</a><br>
> Cc: <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> Subject: RE: [llvm-dev] [RFC][llvm-mca] Adding binary support to llvm-mca.<br>
> <br>
> Just an update to this RFC.  (I thought this was going to be a short email... my<br>
> apologies).<br>
> <br>
> One of the primary limitations described in this RFC (earlier in this thread), is that the<br>
> patch for this RFC only handles linked executables.  This restriction is due to myself<br>
> wanting to avoid handling relocations in a target specific manner, at least in the<br>
> initial patch.  Especially so, since I want to keep the initial patch simple.  However,<br>
> sometimes simple and practical are at odds with each other.  I envision the main use-<br>
> case of llvm-mca, with binary support, is for analyzing .o files.  Probably analyzing .o<br>
> more commonly than fully linked executables.  Without handling relocated objects,<br>
> this patch seems rather useless.<br>
> <br>
> I started exploring an alternative solution this weekend to the aforementioned<br>
> problem.  This alternative solution avoids having to handle relocations, but does give<br>
> us support for object files (with relocated symbols) and linked executables.  The<br>
> change is quite simple, and seems to be effective.  In short, we still generate<br>
> intrinsics as discussed in the RFC, one to mark the start of a code region, and<br>
> another to mark the end.  These intrinsics get lowered into local symbols.  The<br>
> symbols are already  encoded with address information about their position in the<br>
> object file.  What is different is that we ensure that these symbols have unique<br>
> names and also encode the user provided ID value.<br>
> <br>
> Previously the labels  were named like: .Lmca_code_region_start_<number>, and<br>
> similar for .Lmca_code_region_end.  The user id number and region size were<br>
> encoded in the .mca_code_regions object file section.  Previously mca never looked<br>
> at the symbol table.  But, In reality we can calculate the region size by using the<br>
> symbols in the symbol table (look for the mca symbols), instead of relying on the<br>
> information encoded in .mca_code_regions.  The alternative approach gets rid of<br>
> that section entirely but achieves the same functionality by encoding the information<br>
> in the symbol name. In short the alternative approach just parses the symbol table<br>
> for MCA symbols, and the symbol names are encoded with the data we need.<br>
> <br>
> The newly proposed name is formatted<br>
> as: .Lmca_code_region_start.<id>.<function>.<number>.  Similar for<br>
> mca_code_region_end.  'function' is the function that the marker appears in, 'ID' is<br>
> the user-specified ID (this is a value that users specify for easily identifying the code<br>
> region under analysis... just cosmetic), and 'number' is a unique number to avoid any<br>
> duplicate name conflicts.  The benefit of this alternative solution is that we can get<br>
> rid of .mca_code_regions, and gather all of the information llvm-mca needs by<br>
> parsing the symbol table looking for any symbols with the 'mca_code_region_start'<br>
> and 'mca_code_region_end' format discussed above.  Of course, if the string table is<br>
> stripped, then we will lose this data.  The main drawback from this alternative<br>
> approach is that it relies on encoding symbol names and string processing on those<br>
> names.   I'm somewhat biased against doing string parsing, but the code to perform<br>
> this is simple and small, and more importantly it allows llvm-mca to handle linked or<br>
> relocated object files.<br>
> <br>
> -Matt<br>
> <br>
> <br>
> <br>
> > -----Original Message-----<br>
> > From: llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>> On Behalf Of via llvm-dev<br>
> > Sent: Tuesday, December 11, 2018 11:23 AM<br>
> > To: <a href="mailto:llvm-dev@redking.me.uk" target="_blank">llvm-dev@redking.me.uk</a>; <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> > Cc: <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> > Subject: Re: [llvm-dev] [RFC][llvm-mca] Adding binary support to llvm-mca.<br>
> ><br>
> > Thanks for the response Simon.  My reply is inline:<br>
> ><br>
> > > From: Simon Pilgrim <<a href="mailto:llvm-dev@redking.me.uk" target="_blank">llvm-dev@redking.me.uk</a>><br>
> > > Sent: Monday, December 10, 2018 1:40 PM<br>
> > ><br>
> > > Hi Matt,<br>
> > ><br>
> > > I can see a near future where perf-analysis tooling uses branch history<br>
> > > profiler captures to determine how often loops/branches are taken and<br>
> > > feeds that into llvm-mca, especially for hot/branchy loop analysis<br>
> > > reports etc. Are you confident that your approach will be easily<br>
> > > extendable for this?<br>
> ><br>
> > That is a very interesting use case.  The restriction of a code-region to a single<br>
> block<br>
> > is a limitation for any tools that want to analyze branches.  However, I believe<br>
> > that it will be easy to lift this restriction (it's just a check in IR/Verifier).  This<br>
> > limitation is not<br>
> > expressed in the llvm-mca driver.<br>
> ><br>
> > If the information is coming from a profile report, then we'd most<br>
> > likely need to extend the llvm-mca driver to accept profile reports.  Currently,<br>
> > code regions, from the perspective of the  llvm-mca driver, are very simple. They<br>
> are<br>
> > just a collection of MCInst.  The binary support in this RFC+patch<br>
> > disassembles just the address range from marker start address for<br>
> > some specified number of bytes.  It might be useful to add another driver argument<br>
> > so that a user (or tool) can specify, from the command line, a range of instructions<br>
> > to<br>
> > analyze.  I recently added a class for handling inputs to<br>
> > llvm::mca::CodeRegionGenerator,<br>
> > which is just responsible for taking some input and creating a list of MCInst that<br>
> llvm-<br>
> > mca uses.<br>
> > We could subclass this to handle profile reports.<br>
> ><br>
> > > Similarly, being able to generally embed the profile markers in object<br>
> > > libraries for reuse is going to be important for some people - I'd like<br>
> > > to see more of a plan of how this will be achieved. I understand that it<br>
> > > might not be easy for some exe formats.<br>
> ><br>
> > That is definitely a limitation.  This initial patch+RFC only handles linked<br>
> > executables (i.e., the llvm-mca marker symbol addresses are resolved).<br>
> > I'm working on a better solution so that this will not be a restriction.<br>
> > In fact, I'll probably delay trying to land any patches until I solve relocations<br>
> > (or use a different solution for identifying start/end addresses for llvm-mca code<br>
> > regions).<br>
> ><br>
> > > Sorry if I'm being too critical, but I'm a bit worried that we end up<br>
> > > with an initial implementation that will take a lot of reworking to meet<br>
> > > our final aims.<br>
> > ><br>
> > > Thanks, Simon.<br>
> ><br>
> > I understand your criticisms and value your input. Thanks a ton!<br>
> ><br>
> > -Matt<br>
> ><br>
> ><br>
> > > On 10/12/2018 19:32, Matt Davis wrote:<br>
> > > > Thanks for the feedback Guillaume and Clement!<br>
> > > ><br>
> > > > In response to Clement:<br>
> > > ><br>
> > > >>> In terms of future-proofness of only allowing regions within a basic<br>
> > > >>> block, are we confident we can actually ever simulate branches apart from<br>
> > > >>> "always taken, perfectly predicated" loop ? Even this simple need requires<br>
> > > >>> knowing quite a few details on the frontend. The current design could<br>
> > > >>> handle this use case with the addition of an external "loop mode" option to<br>
> > > >>> MCA. If there are no other strong use cases, I would advocate for<br>
> > > >>> experimental intrinsics unless people can contribute other example use<br>
> > > >>> cases.<br>
> > > > In short, I am in agreement and think that handling of branching or loop<br>
> > > > constructs should be isolated to the llvm-mca driver/front-end.  The<br>
> > > > only thing the code regions should be concerned with is identifying<br>
> > > > blocks of instructions that will later be used by the front end.<br>
> > > ><br>
> > > > We can place limitations to how those blocks are formed. For example the<br>
> > > > current implementation forces regions to be isolated to a single basic<br>
> > > > block.  However, we anticipate lifting this restriction once branching<br>
> > > > is handled.<br>
> > > ><br>
> > > > -Matt<br>
> > > ><br>
> > > ><br>
> > > > On Mon, Dec 10, 2018 at 04:15:46PM +0100, Guillaume Chatelet wrote:<br>
> > > >> +1 to what Clement said.<br>
> > > >> I believe the intrinsics are a better design to support many architectures.<br>
> > > >><br>
> > > >> IACA users are probably decorating their code with IACA_START / IACA_END<br>
> > > >> macros. One possibility is to provide a header that define these macros in<br>
> > > >> terms of the new intrinsics.<br>
> > > >><br>
> > > >> On Mon, Dec 10, 2018 at 3:59 PM Clement Courbet <<a href="mailto:courbet@google.com" target="_blank">courbet@google.com</a>><br>
> > > wrote:<br>
> > > >><br>
> > > >>> Hi Matt/Andrea,<br>
> > > >>><br>
> > > >>> I see pros and cons for IACA-style markers vs intrinsics.<br>
> > > >>> On the one hand, IACA-style markers are very magical, and not very visible<br>
> > > >>> in both the source and object code. Using IACA-style markers has the<br>
> > > >>> advantage that you can use llvm-mca as a drop-in replacement for IACA, or<br>
> > > >>> even to compare their outputs on the exact same binary. They also do not<br>
> > > >>> require tooling on the compiler side and allow comparing the output of<br>
> > > >>> several compilers.<br>
> > > >>><br>
> > > >> On the other hand, IACA-style markers do not have a equivalent on other<br>
> > > >>> architectures, and I'm not sure inventing new ones is a good idea :) I<br>
> > > >>> think the latter makes them pretty much a no-go for llvm-mca as I don't<br>
> > > >>> think we'll want to teach each target how to parse code regions. That's<br>
> > > >>> much better handled in a target-agnostic way by the object. Intel got away<br>
> > > >>> with them because they only had to support one architecture.<br>
> > > >>><br>
> > > >>> tl;dr: In the case of llvm-mca, I like your design better than the markers.<br>
> > > >>><br>
> > > >>> In terms of future-proofness of only allowing regions within a basic<br>
> > > >>> block, are we confident we can actually ever simulate branches apart from<br>
> > > >>> "always taken, perfectly predicated" loop ? Even this simple need requires<br>
> > > >>> knowing quite a few details on the frontend. The current design could<br>
> > > >>> handle this use case with the addition of an external "loop mode" option to<br>
> > > >>> MCA. If there are no other strong use cases, I would advocate for<br>
> > > >>> experimental intrinsics unless people can contribute other example use<br>
> > > >>> cases.<br>
> > > >>><br>
> > > >>> On Mon, Dec 3, 2018 at 11:38 PM Matt Davis <<a href="mailto:matthew.davis@sony.com" target="_blank">matthew.davis@sony.com</a>><br>
> > > wrote:<br>
> > > >>><br>
> > > >>>> Hi Andrea,<br>
> > > >>>><br>
> > > >>>> On Mon, Dec 03, 2018 at 01:21:33PM +0000, Andrea Di Biagio wrote:<br>
> > > >>>>> So, I have been thinking a bit more about this whole design.<br>
> > > >>>>><br>
> > > >>>>> The more I think about your suggested design, the more I am convinced<br>
> > > >>>> that<br>
> > > >>>>> we should do something more to support ranges in binary object files<br>
> > > >>>> too.<br>
> > > >>>>> My understanding is that the reason why we don't support object files in<br>
> > > >>>>> general, is because of the presence of relocations. That is because a<br>
> > > >>>>> region start marker is effectively symbol relative, and the symbol (a<br>
> > > >>>>> function) would be relocated in the final executable.<br>
> > > >>>>> You mentioned to me that resolving even a 'simple' symbol-relative<br>
> > > >>>>> relocation is not trivial, beause it requires specific knowledge about<br>
> > > >>>> the<br>
> > > >>>>> binary format, and the target (i.e. how relocations are encoded is<br>
> > > >>>> target<br>
> > > >>>>> specific). I am surprised that there is not a utility library for<br>
> > > >>>> resolving<br>
> > > >>>>> relocations.. but I am not familiar with that part of the compiler. I<br>
> > > >>>> was<br>
> > > >>>>> hoping that there was a target specific interface to use in this case...<br>
> > > >>>> There might be a better way of resolving the relocs, but from what I saw<br>
> > > >>>> looking at llvm-objdump and other related tools, it seems that resolving<br>
> > > >>>> the relocated symbol is a target specific effort.  I also spent sometime<br>
> > > >>>> sniffing around ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp which also<br>
> > > >>>> performs the reloc resolution.  I should clarify that I too am not an<br>
> > > >>>> expert in llvm's utilities for performing symbol/reloc resolution, and<br>
> > > >>>> perhaps someone in the community can point me in the right direction.  I<br>
> > > >>>> can clearly see the reloc data in the object file via tools like<br>
> > > >>>> objdump; however, accessing the relocs via<br>
> > > >>>> llvm::object::ObjectFile::relocations() did not produce address values<br>
> > > >>>> that we could use (values of zero).<br>
> > > >>>><br>
> > > >>>> I was hoping that, for a first pass at this patch, supporting just<br>
> > > >>>> executables would be okay.  That keeps this initial patch set simple,<br>
> > > >>>> and hopefully will encourage others to take a peek at it, since it's<br>
> > > >>>> less daunting than what it might otherwise be.  Of course, there is the<br>
> > > >>>> concern that this initial patch will lock us into a design that will be<br>
> > > >>>> more complicated to unravel later.<br>
> > > >>>><br>
> > > >>>>> An alternative approach would require that you define your own<br>
> > > >>>>> "symbol-relative" reference. After all, ranges are just a sequences of<br>
> > > >>>>> instructions in a function. If a function symbol is described by the<br>
> > > >>>> symbol<br>
> > > >>>>> table, then you should be able to obtain its offset in the .text<br>
> > > >>>> section.<br>
> > > >>>>> So, you could potentially encode your own symbol+offset. However, the<br>
> > > >>>>> linker would not be able to understand your "custom relocation", and<br>
> > > >>>>> information about regions in the final elf would be basically broken.<br>
> > > >>>>> So,that would not be a solution...<br>
> > > >>>>><br>
> > > >>>>> I don't know honestly what is the best approach to use in this case.<br>
> > > >>>>> As a compromise, it would not be a bad idea to add the ability to<br>
> > > >>>> specify<br>
> > > >>>>> ranges from command line. What do you think?<br>
> > > >>>>> Still, from a user point of view, the idea that we don't support object<br>
> > > >>>>> files in general sounds like a big limitation.<br>
> > > >>>> I agree, only supporting executables is a limitation.  However, I'd<br>
> > > >>>> like to land the base support now and add in the additional<br>
> > > >>>> features/support after this large patch set lands.  But I can see<br>
> > > >>>> where landing the whole thing entirely also makes sense.<br>
> > > >>>><br>
> > > >>>>> About the new experimental intrinsics: those would definitely work well<br>
> > > >>>> for<br>
> > > >>>>> the simple case where instructions are from the same basic block.<br>
> > > >>>>> However, some/most of the constraints that you plan to add will have to<br>
> > > >>>>> change if in future we decide to allow ranges that potentially cross<br>
> > > >>>>> multiple basic blocks. How will the rules/constraints on those new<br>
> > > >>>>> intrinsics change? I just want to make sure that the suggested design is<br>
> > > >>>>> future-proof.<br>
> > > >>>> Since the llvm/clang parts of the code are just responsible for<br>
> > > >>>> collecting where a range starts/ends, I hope that we can remove some<br>
> > > >>>> of the baked-in constraints that are specified in IR/Verifier.cpp.<br>
> > > >>>> As you pointed out earlier in this thread, we might want to<br>
> > > >>>> introduce a dominance check if/when we lift the one-basic-block<br>
> > > >>>> restriction.<br>
> > > >>>><br>
> > > >>>> -Matt<br>
> > > >>>><br>
> > > >>>>> -Andrea<br>
> > > >>>>><br>
> > > >>>>> On Tue, Nov 27, 2018 at 5:08 PM Andrea Di Biagio <<br>
> > > >>>> <a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>><br>
> > > >>>>> wrote:<br>
> > > >>>>><br>
> > > >>>>>> Thanks for clarifying it Matt.<br>
> > > >>>>>><br>
> > > >>>>>> In general, I quite like your suggested design.<br>
> > > >>>>>><br>
> > > >>>>>> My only concern is about the semantic of the two new intrinsics. You<br>
> > > >>>>>> design doesn't allow mca ranges to span through multiple basic<br>
> > > >>>> blocks. That<br>
> > > >>>>>> constraint is acceptable for now, since llvm-mca doesn't know how to<br>
> > > >>>> deal<br>
> > > >>>>>> with control flow.<br>
> > > >>>>>> However, I am a bit concerned about what might happen in future if we<br>
> > > >>>>>> decide to let users specify code regions that span through multiple<br>
> > > >>>> basic<br>
> > > >>>>>> blocks. Basically, I don't particularly like the idea of changing the<br>
> > > >>>>>> semantic of already existing intrinsic. A design that already<br>
> > > >>>> accounts for<br>
> > > >>>>>> that particular scenario/future work would be ideal. That being said,<br>
> > > >>>>>> marking those new intrinsics as 'experimental' may be a good<br>
> > > >>>> compromise (at<br>
> > > >>>>>> least for now).<br>
> > > >>>>>><br>
> > > >>>>>> So, I am quite happy overall with the direction of this RFC.<br>
> > > >>>>>> However, I am interesting to hear from other developers about your<br>
> > > >>>>>> suggested design.<br>
> > > >>>>>><br>
> > > >>>>>>> This initial patch only targets ELF object files, and does not<br>
> > > >>>> handle<br>
> > > >>>>>> relocatable addresses. Since the start of a code region is<br>
> > > >>>> represented as<br>
> > > >>>>>> an<br>
> > > >>>>>> assembly label, and referenced in the .mca_code_regions section, that<br>
> > > >>>>>> address<br>
> > > >>>>>> is relocatable.<br>
> > > >>>>>><br>
> > > >>>>>> This may be okay for now. However, it would be nice to remove that<br>
> > > >>>>>> constraint in future and add support to generic object files.<br>
> > > >>>>>><br>
> > > >>>>>> -Andrea<br>
> > > >>>>>><br>
> > > >>>>>> On Thu, Nov 22, 2018 at 7:21 PM <<a href="mailto:Matthew.Davis@sony.com" target="_blank">Matthew.Davis@sony.com</a>> wrote:<br>
> > > >>>>>><br>
> > > >>>>>>> I want to clarify a few restrictions of llvm-mca code regions that<br>
> > > >>>> this<br>
> > > >>>>>>> RFC proposes:<br>
> > > >>>>>>><br>
> > > >>>>>>> 1) All llvm-mca code regions must start with an<br>
> > > >>>>>>> llvm.mca.code.region.start intrinsic and end with<br>
> > > >>>>>>> an llvm.mca.code.region.end intrinsic.  This rule is enforced at the<br>
> > > >>>> IR<br>
> > > >>>>>>> level in the IR verifier.<br>
> > > >>>>>>><br>
> > > >>>>>>> 2) llvm-mca code regions cannot nest.  This restriction implies that<br>
> > > >>>> an<br>
> > > >>>>>>> llvm.mca.code.region.start<br>
> > > >>>>>>> must have a llvm.mca.code.region.end intrinsic without any other<br>
> > > >>>> llvm.mca<br>
> > > >>>>>>> start intrinsics<br>
> > > >>>>>>> between the two. The current implementation in the patch enforces<br>
> > > >>>> this<br>
> > > >>>>>>> restriction at the<br>
> > > >>>>>>> IR level via the IR Verifier.<br>
> > > >>>>>>><br>
> > > >>>>>>> 3) An llvm-mca code region cannot span multiple basic blocks.<br>
> > > >>>> llvm-mca<br>
> > > >>>>>>> does not follow<br>
> > > >>>>>>> branches (yet).  Instead, a branch instruction is treated by llvm-mca<br>
> > > >>>>>>> like any other instruction.<br>
> > > >>>>>>> The current patch associated with this RFC does not enforce this<br>
> > > >>>>>>> restriction.  I plan on updating<br>
> > > >>>>>>> the patch to enforce that a code region can only belong to a single<br>
> > > >>>> basic<br>
> > > >>>>>>> block.  This is a simple<br>
> > > >>>>>>> check, ensuring that both the llvm.mca.code.region.start and<br>
> > > >>>> accompanying<br>
> > > >>>>>>> end intrinsics live<br>
> > > >>>>>>> in the same basic block. I imagine adding this check at the IR level<br>
> > > >>>> when<br>
> > > >>>>>>> we also verify points 1 and 2<br>
> > > >>>>>>> above.  That will keep the code-region verification logic isolated<br>
> > > >>>> to the<br>
> > > >>>>>>> IR verifier.  The start/end<br>
> > > >>>>>>> intrinsics should not have any uses, so I'm not sure that they would<br>
> > > >>>> be<br>
> > > >>>>>>> moved/sunk on behalf<br>
> > > >>>>>>> of any other instruction.  In other words, I do not imagine that a<br>
> > > >>>> start<br>
> > > >>>>>>> and end would be split<br>
> > > >>>>>>> apart due to later MI optimizations.  If I discover that such a case<br>
> > > >>>>>>> occurs, then I might add the<br>
> > > >>>>>>> basic-block check prior to emitting the code region data to the<br>
> > > >>>> object<br>
> > > >>>>>>> file.    Once  llvm-mca  is<br>
> > > >>>>>>> updated to handle branches, then we can remove this constraint.<br>
> > > >>>>>>><br>
> > > >>>>>>> -Matt<br>
> > > >>>>>>><br>
> > > >>>>>>>> -----Original Message-----<br>
> > > >>>>>>>> From: llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>> On Behalf Of Matt<br>
> > > >>>>>>> Davis via llvm-<br>
> > > >>>>>>>> dev<br>
> > > >>>>>>>> Sent: Wednesday, November 21, 2018 8:47 AM<br>
> > > >>>>>>>> To: Andrea Di Biagio <<a href="mailto:andrea.dibiagio@gmail.com" target="_blank">andrea.dibiagio@gmail.com</a>><br>
> > > >>>>>>>> Cc: llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>; Di Biagio, Andrea<br>
> > > >>>>>>>> <<a href="mailto:Andrea.Dibiagio@sony.com" target="_blank">Andrea.Dibiagio@sony.com</a>>; <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> > > >>>>>>>> Subject: Re: [llvm-dev] [RFC][llvm-mca] Adding binary support to<br>
> > > >>>>>>> llvm-mca.<br>
> > > >>>>>>>> Hi Andrea,<br>
> > > >>>>>>>><br>
> > > >>>>>>>> Thanks for your input.<br>
> > > >>>>>>>><br>
> > > >>>>>>>> On Wed, Nov 21, 2018 at 12:43:52PM +0000, Andrea Di Biagio wrote:<br>
> > > >>>>>>>> [... snip ...]<br>
> > > >>>>>>>>> About the suggested design:<br>
> > > >>>>>>>>> I like the idea of being able to identify code regions using a<br>
> > > >>>> numeric<br>
> > > >>>>>>>>> identifier.<br>
> > > >>>>>>>>> However, what happens if a code region spans through multiple<br>
> > > >>>> basic<br>
> > > >>>>>>> blocks?<br>
> > > >>>>>>>> The current patch does not take into consideration cases where the<br>
> > > >>>>>>>> region start and end intrinsics are placed in different basic<br>
> > > >>>> blocks.<br>
> > > >>>>>>>> Such would be the case if a region is defined to span multiple<br>
> > > >>>> blocks.<br>
> > > >>>>>>>> This would be similar to the current case where a user places a<br>
> > > >>>>>>>> #LLVM-MCA-BEGIN assembly comment in one block and an #LLVM-<br>
> > MCA-<br>
> > > END<br>
> > > >>>> in<br>
> > > >>>>>>>> another.  However, as you point out below, if the user does this<br>
> > > >>>> in the<br>
> > > >>>>>>>> source code via intrinsics (just what this patch is proposing),<br>
> > > >>>> then<br>
> > > >>>>>>>> there is a chance that optimizations might change the layout of the<br>
> > > >>>>>>>> instructions and confuse the ordering of the MCA intrinsics.<br>
> > > >>>>>>>><br>
> > > >>>>>>>> Since MCA does not follow branches (MCA just treats a branch as it<br>
> > > >>>> would<br>
> > > >>>>>>>> a non-branching instruction), it seems that a user should be aware<br>
> > > >>>> that<br>
> > > >>>>>>>> defining MCA code regions that span multiple blocks might result<br>
> > > >>>> in an<br>
> > > >>>>>>>> unexpected analysis.  While we do not discourage this, it seems<br>
> > > >>>> like<br>
> > > >>>>>>>> such a case will probably not produce an expected result for the<br>
> > > >>>> user.<br>
> > > >>>>>>>> We could introduce a warning, or automatically divide the regions<br>
> > > >>>> so<br>
> > > >>>>>>>> that a single region can only contain a single block.<br>
> > > >>>>>>>><br>
> > > >>>>>>>>> My understanding is that code regions are not allowed to<br>
> > > >>>> overlap. So,<br>
> > > >>>>>>> it<br>
> > > >>>>>>>>> makes sense if ` __mca_code_region_end()` doesn't take an ID as<br>
> > > >>>> input.<br>
> > > >>>>>>>>> However, what if ` __mca_code_region_end()` ends in a different<br>
> > > >>>> basic<br>
> > > >>>>>>> block?<br>
> > > >>>>>>>>> `__mca_code_region_start()` has to always dominate `<br>
> > > >>>>>>>>> __mca_code_region_end()`. This is trivial to verify when both<br>
> > > >>>> calls<br>
> > > >>>>>>> are in<br>
> > > >>>>>>>>> a same basic block; however, we need to make sure that the<br>
> > > >>>>>>> relationship is<br>
> > > >>>>>>>>> still the same when the `end()` call is in a different basic<br>
> > > >>>> block.<br>
> > > >>>>>>>>> That would not be enough. I think we should also verify  that `<br>
> > > >>>>>>>>> __mca_code_region_end()` always post-dominates the call to<br>
> > > >>>>>>>>> `__mca_code_region_start()`.<br>
> > > >>>>>>>> In any case this patch should probably check dominance of the<br>
> > > >>>>>>>> intrinsics, even though MCA does not follow branches and MCA does<br>
> > > >>>> not<br>
> > > >>>>>>>> not explicitly forbid a region from containing multiple blocks.<br>
> > > >>>>>>>><br>
> > > >>>>>>>>> My question is: what happens with basic block reordering? We<br>
> > > >>>> don't<br>
> > > >>>>>>> know the<br>
> > > >>>>>>>>> layout of basic blocks until we reach code emission. How does it<br>
> > > >>>> work<br>
> > > >>>>>>> for<br>
> > > >>>>>>>>> regions that span through multiple basic blocks?. I think your<br>
> > > >>>> RFC<br>
> > > >>>>>>> should<br>
> > > >>>>>>>>> clarify this aspect.<br>
> > > >>>>>>>>><br>
> > > >>>>>>>>> As a side note: at the moment, llvm-mca doesn't know how to deal<br>
> > > >>>> with<br>
> > > >>>>>>>>> branches. So, for simplicity we could force code regions to only<br>
> > > >>>>>>> contain<br>
> > > >>>>>>>>> instructions from a single basic block.<br>
> > > >>>>>>>>><br>
> > > >>>>>>>>> However, In future we may want to teach llvm-mca how to analyze<br>
> > > >>>>>>> branchy<br>
> > > >>>>>>>>> code too. For example, we could introduce a simple control-flow<br>
> > > >>>>>>> analysis in<br>
> > > >>>>>>>>> llvm-mca, and use an external "branch trace" information (for<br>
> > > >>>>>>> example, a<br>
> > > >>>>>>>>> perf trace generated by an external tool) to decorate branches<br>
> > > >>>> with<br>
> > > >>>>>>> with<br>
> > > >>>>>>>>> branch probabilities (similarly to what we currently do in LLVM<br>
> > > >>>> with<br>
> > > >>>>>>> PGO).<br>
> > > >>>>>>>>> We could then use that knowledge to model branch prediction and<br>
> > > >>>>>>> simulate<br>
> > > >>>>>>>>> what happens in the presence of multiple branches.<br>
> > > >>>>>>>>><br>
> > > >>>>>>>>> So, the idea of having regions that potentially span multiple<br>
> > > >>>> basic<br>
> > > >>>>>>> blocks<br>
> > > >>>>>>>>> is not bad in general. However, I think you should better clarify<br>
> > > >>>>>>> what are<br>
> > > >>>>>>>>> the constraints (at least, you should answer to my questions from<br>
> > > >>>>>>> before).<br>
> > > >>>>>>>> I agree! Thanks for pointing that out.<br>
> > > >>>>>>>><br>
> > > >>>>>>>>> If we decide to use those new intrinsics, then those should be<br>
> > > >>>>>>> experimental<br>
> > > >>>>>>>>> (at least to start).<br>
> > > >>>>>>>> Agreed.<br>
> > > >>>>>>>><br>
> > > >>>>>>>> -Matt<br>
> > > >>>>>>>><br>
> > > >>>>>>>>> On Thu, Nov 15, 2018 at 11:07 PM via llvm-dev <<br>
> > > >>>>>>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
> > > >>>>>>>>> wrote:<br>
> > > >>>>>>>>><br>
> > > >>>>>>>>>> Introduction<br>
> > > >>>>>>>>>> -----------------<br>
> > > >>>>>>>>>> Currently llvm-mca only accepts assembly code as input. We<br>
> > > >>>> would<br>
> > > >>>>>>> like to<br>
> > > >>>>>>>>>> extend llvm-mca to support object files, allowing users to<br>
> > > >>>> analyze<br>
> > > >>>>>>> the<br>
> > > >>>>>>>>>> performance of binaries. The proposed changes (which involve<br>
> > > >>>> both<br>
> > > >>>>>>>>>> clang and llvm) optionally introduce an object file section,<br>
> > > >>>> but<br>
> > > >>>>>>> this can<br>
> > > >>>>>>>>>> be<br>
> > > >>>>>>>>>> stripped-out if desired.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> For the llvm-mca binary support feature to be useful, a user<br>
> > > >>>> needs<br>
> > > >>>>>>> to tell<br>
> > > >>>>>>>>>> llvm-mca which portions of their code they would like analyzed.<br>
> > > >>>>>>> Currently,<br>
> > > >>>>>>>>>> this is accomplished via assembly comments. However, assembly<br>
> > > >>>>>>> comments are<br>
> > > >>>>>>>>>> not<br>
> > > >>>>>>>>>> preserved in object files, and this has encouraged this RFC.<br>
> > > >>>> For the<br>
> > > >>>>>>>>>> proposed<br>
> > > >>>>>>>>>> binary support, we need to introduce changes to clang and llvm<br>
> > > >>>> to<br>
> > > >>>>>>> allow the<br>
> > > >>>>>>>>>> user's object code to be recognized by llvm-mca:<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> * We need a way for a user to identify a region/block of code<br>
> > > >>>> they<br>
> > > >>>>>>> want<br>
> > > >>>>>>>>>>     analyzed by llvm-mca.<br>
> > > >>>>>>>>>> * We need the information defining the user's region of code<br>
> > > >>>> to be<br>
> > > >>>>>>>>>> maintained<br>
> > > >>>>>>>>>>     in the object file so that llvm-mca can analyze the desired<br>
> > > >>>>>>> region(s)<br>
> > > >>>>>>>>>> from the<br>
> > > >>>>>>>>>>     object file.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> We define a "code region" as a subset of a user's program that<br>
> > > >>>> is<br>
> > > >>>>>>> to be<br>
> > > >>>>>>>>>> analyzed via llvm-mca. The sequence of instructions to be<br>
> > > >>>> analyzed<br>
> > > >>>>>>> is<br>
> > > >>>>>>>>>> represented as a pair: <start, end> where the 'start' marks the<br>
> > > >>>>>>> beginning<br>
> > > >>>>>>>>>> of<br>
> > > >>>>>>>>>> the user's source code and 'end' terminates the sequence. The<br>
> > > >>>>>>> instructions<br>
> > > >>>>>>>>>> between 'start' and 'end' form the region that can be analyzed<br>
> > > >>>> by<br>
> > > >>>>>>> llvm-mca<br>
> > > >>>>>>>>>> at a<br>
> > > >>>>>>>>>> later time.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> Example<br>
> > > >>>>>>>>>> -----------<br>
> > > >>>>>>>>>> Before we go into the details of this proposed change, let's<br>
> > > >>>> first<br>
> > > >>>>>>> look at<br>
> > > >>>>>>>>>> a<br>
> > > >>>>>>>>>> simple example:<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> // example.c -- Analyze a dot-product expression.<br>
> > > >>>>>>>>>> double test(double x, double y) {<br>
> > > >>>>>>>>>>     double result = 0.0;<br>
> > > >>>>>>>>>>     __mca_code_region_start(42);<br>
> > > >>>>>>>>>>     result += x * y;<br>
> > > >>>>>>>>>>     __mca_code_region_end();<br>
> > > >>>>>>>>>>     return result;<br>
> > > >>>>>>>>>> }<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> In the example above, we have identified a code region, in this<br>
> > > >>>>>>> case a<br>
> > > >>>>>>>>>> single<br>
> > > >>>>>>>>>> dot-product expression. For the sake of brevity and simplicity,<br>
> > > >>>>>>> we've<br>
> > > >>>>>>>>>> chosen<br>
> > > >>>>>>>>>> a very simple example, but in reality a more complicated<br>
> > > >>>> example<br>
> > > >>>>>>> could use<br>
> > > >>>>>>>>>> multiple expressions. We have also denoted this region as<br>
> > > >>>> number<br>
> > > >>>>>>> 42. That<br>
> > > >>>>>>>>>> identifier is only for the user, and simplifies reading an<br>
> > > >>>> llvm-mca<br>
> > > >>>>>>>>>> analysis<br>
> > > >>>>>>>>>> report later.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> When this code is compiled, the region markers (the<br>
> > > >>>> mca_code_region<br>
> > > >>>>>>>>>> markers)<br>
> > > >>>>>>>>>> are transformed into assembly labels. While the markers are<br>
> > > >>>>>>> presented as<br>
> > > >>>>>>>>>> function calls, in reality they are no-ops.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> test:<br>
> > > >>>>>>>>>> pushq   %rbp<br>
> > > >>>>>>>>>> movq    %rsp, %rbp<br>
> > > >>>>>>>>>> movsd   %xmm0, -8(%rbp)<br>
> > > >>>>>>>>>> movsd   %xmm1, -16(%rbp)<br>
> > > >>>>>>>>>> .Lmca_code_region_start_0: # LLVM-MCA-START ID: 42<br>
> > > >>>>>>>>>> xorps   %xmm0, %xmm0<br>
> > > >>>>>>>>>> movsd   %xmm0, -24(%rbp)<br>
> > > >>>>>>>>>> movsd   -8(%rbp), %xmm0<br>
> > > >>>>>>>>>> mulsd   -16(%rbp), %xmm0<br>
> > > >>>>>>>>>> addsd   -24(%rbp), %xmm0<br>
> > > >>>>>>>>>> movsd   %xmm0, -24(%rbp)<br>
> > > >>>>>>>>>> .Lmca_code_region_end_0: # LLVM-MCA-END ID: 42<br>
> > > >>>>>>>>>> movsd   -24(%rbp), %xmm0<br>
> > > >>>>>>>>>> popq    %rbp<br>
> > > >>>>>>>>>> retq<br>
> > > >>>>>>>>>> .section        .mca_code_regions,"",@progbits<br>
> > > >>>>>>>>>> .quad   42<br>
> > > >>>>>>>>>> .quad   .Lmca_code_region_start_0<br>
> > > >>>>>>>>>> .quad   .Lmca_code_region_end_0-.Lmca_code_region_start_0<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> The assembly has been trimmed to show the portions relevant to<br>
> > > >>>> this<br>
> > > >>>>>>> RFC.<br>
> > > >>>>>>>>>> Notice the labels enclose the user's defined region, and that<br>
> > > >>>> they<br>
> > > >>>>>>>>>> preserve the<br>
> > > >>>>>>>>>> user's arbitrary region identifier, the ever-so-important<br>
> > > >>>> region 42.<br>
> > > >>>>>>>>>> In the object file section .mca_code_regions, we have noted the<br>
> > > >>>>>>> user's<br>
> > > >>>>>>>>>> region<br>
> > > >>>>>>>>>> identifier (.quad 42), start address, and region size. A more<br>
> > > >>>>>>> complicated<br>
> > > >>>>>>>>>> example can have multiple regions defined within a single<br>
> > > >>>>>>> .mca_code_regions<br>
> > > >>>>>>>>>> section. This section can be read by llvm-mca, allowing<br>
> > > >>>> llvm-mca to<br>
> > > >>>>>>> take<br>
> > > >>>>>>>>>> object files as input instead of assembly source.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> Details<br>
> > > >>>>>>>>>> ---------<br>
> > > >>>>>>>>>> We need a way for a user to identify a region/block of code<br>
> > > >>>> they<br>
> > > >>>>>>> want<br>
> > > >>>>>>>>>> analyzed<br>
> > > >>>>>>>>>> by llvm-mca. We solve this problem by introducing two<br>
> > > >>>> intrinsics<br>
> > > >>>>>>> that a<br>
> > > >>>>>>>>>> user can<br>
> > > >>>>>>>>>> specify, for identifying regions of code for analysis.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> The two intrinsics are: llvm.mca.code.regions.start and<br>
> > > >>>>>>>>>> llvm.mca.code.regions.end. A user can identify a code region by<br>
> > > >>>>>>> inserting<br>
> > > >>>>>>>>>> the<br>
> > > >>>>>>>>>> mca_code_region_start and mca_code_region_end markers. These<br>
> > > >>>> are<br>
> > > >>>>>>> simply<br>
> > > >>>>>>>>>> clang builtins and are transformed into the aforementioned<br>
> > > >>>>>>> intrinsics<br>
> > > >>>>>>>>>> during<br>
> > > >>>>>>>>>> compilation. The code between the intrinsics are what we call<br>
> > > >>>> "code<br>
> > > >>>>>>>>>> regions"<br>
> > > >>>>>>>>>> and are to be easily identifiable by llvm-mca; any code<br>
> > > >>>> between a<br>
> > > >>>>>>> start/end<br>
> > > >>>>>>>>>> pair can be analyzed by llvm-mca at a later time. A user can<br>
> > > >>>> define<br>
> > > >>>>>>>>>> multiple<br>
> > > >>>>>>>>>> non-overlapping code regions within their program.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> The llvm.mca.code.region.start intrinsic takes an integer<br>
> > > >>>> constant<br>
> > > >>>>>>> as its<br>
> > > >>>>>>>>>> only<br>
> > > >>>>>>>>>> argument. This argument is implemented as a metadata i32, and<br>
> > > >>>> is<br>
> > > >>>>>>> only used<br>
> > > >>>>>>>>>> when generating llvm-mca reports. This value allows a user to<br>
> > > >>>> more<br>
> > > >>>>>>> easily<br>
> > > >>>>>>>>>> identify a specific code region. llvm.mca.code.region.end<br>
> > > >>>> takes no<br>
> > > >>>>>>>>>> arguments.<br>
> > > >>>>>>>>>> Since we disallow nesting of regions, the first 'end' intrinsic<br>
> > > >>>>>>> lexically<br>
> > > >>>>>>>>>> following a 'start' intrinsic represents the end of that code<br>
> > > >>>>>>> region.<br>
> > > >>>>>>>>>> Now that we have a solution for identifying regions for<br>
> > > >>>> analysis,<br>
> > > >>>>>>> we now<br>
> > > >>>>>>>>>> need a<br>
> > > >>>>>>>>>> way for preserving that information to be read at a later<br>
> > > >>>> time. To<br>
> > > >>>>>>>>>> accomplish<br>
> > > >>>>>>>>>> this we propose adding a new section (.mca_code_regions) to the<br>
> > > >>>>>>> object file<br>
> > > >>>>>>>>>> generated by llvm. During code generation, the start/end<br>
> > > >>>> intrinsics<br>
> > > >>>>>>>>>> described<br>
> > > >>>>>>>>>> above will be transformed into start/end labels in assembly.<br>
> > > >>>> When<br>
> > > >>>>>>> llvm<br>
> > > >>>>>>>>>> generates the object file from the user's code, these start/end<br>
> > > >>>>>>> labels<br>
> > > >>>>>>>>>> form a<br>
> > > >>>>>>>>>> pair of values identifying the start of the user's code<br>
> > > >>>> region, and<br>
> > > >>>>>>> size.<br>
> > > >>>>>>>>>> The<br>
> > > >>>>>>>>>> size represents the number of bytes between the start and end<br>
> > > >>>>>>> address of<br>
> > > >>>>>>>>>> the<br>
> > > >>>>>>>>>> labels. Note that the labels are emitted during assembly<br>
> > > >>>> printing.<br>
> > > >>>>>>> We hope<br>
> > > >>>>>>>>>> that these labels have no influence on code generation or<br>
> > > >>>>>>> basic-block<br>
> > > >>>>>>>>>> placement. However, the target assembler strategy for handling<br>
> > > >>>>>>> labels is<br>
> > > >>>>>>>>>> outside of our control.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> This proposed change affects the size of a binary, but only if<br>
> > > >>>> the<br>
> > > >>>>>>> user<br>
> > > >>>>>>>>>> calls<br>
> > > >>>>>>>>>> the start/end builtins mentioned above. The additional size of<br>
> > > >>>> the<br>
> > > >>>>>>>>>> .mca_code_regions section, which we imagine to be very small<br>
> > > >>>> (to<br>
> > > >>>>>>> the order<br>
> > > >>>>>>>>>> of a<br>
> > > >>>>>>>>>> few bytes), can trivially be stripped by tools like 'strip' or<br>
> > > >>>>>>> 'objcopy'.<br>
> > > >>>>>>>>>> Implementation Status<br>
> > > >>>>>>>>>> ------------------------------<br>
> > > >>>>>>>>>> We currently have the proposed changes implemented at the url<br>
> > > >>>>>>> posted below.<br>
> > > >>>>>>>>>> This initial patch only targets ELF object files, and does not<br>
> > > >>>>>>> handle<br>
> > > >>>>>>>>>> relocatable addresses. Since the start of a code region is<br>
> > > >>>>>>> represented as<br>
> > > >>>>>>>>>> an<br>
> > > >>>>>>>>>> assembly label, and referenced in the .mca_code_regions<br>
> > > >>>> section,<br>
> > > >>>>>>> that<br>
> > > >>>>>>>>>> address<br>
> > > >>>>>>>>>> is relocatable. That value can be represented as<br>
> > > >>>> section-relative<br>
> > > >>>>>>>>>> relocatable<br>
> > > >>>>>>>>>> symbol (.text + addend), but we are not handling that case yet.<br>
> > > >>>>>>> Instead,<br>
> > > >>>>>>>>>> the<br>
> > > >>>>>>>>>> proposed changes only handle linked/executable object files.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> For purposes of review and to communicate the idea, the change<br>
> > > >>>> is<br>
> > > >>>>>>>>>> presented as a monolithic patch here:<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> <a href="https://reviews.llvm.org/D54603" rel="noreferrer" target="_blank">https://reviews.llvm.org/D54603</a><br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> The change is presented as a monolithic patch; however, if<br>
> > > >>>> accepted<br>
> > > >>>>>>>>>> the patch will be split into three smaller patches:<br>
> > > >>>>>>>>>> 1. The introduction of the builtins to clang.<br>
> > > >>>>>>>>>> 2. The llvm portion (the added intrinsics).<br>
> > > >>>>>>>>>> 3. The llvm-mca portion.<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> Thanks!<br>
> > > >>>>>>>>>><br>
> > > >>>>>>>>>> -Matt<br>
> > > >>>>>>>>>> _______________________________________________<br>
> > > >>>>>>>>>> LLVM Developers mailing list<br>
> > > >>>>>>>>>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> > > >>>>>>>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
> > > >>>>>>>>>><br>
> > > >>>>>>>> _______________________________________________<br>
> > > >>>>>>>> LLVM Developers mailing list<br>
> > > >>>>>>>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> > > >>>>>>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
> > _______________________________________________<br>
> > LLVM Developers mailing list<br>
> > <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>