[llvm] r280442 - [CFGPrinter] Display branch weight on the edges

Adam Nemet via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 2 10:29:52 PDT 2016


> On Sep 1, 2016, at 11:01 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> Adam Nemet via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> writes:
>> Author: anemet
>> Date: Thu Sep  1 19:28:26 2016
>> New Revision: 280442
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=280442&view=rev
>> Log:
>> [CFGPrinter] Display branch weight on the edges
>> 
>> Summary:
>> This is pretty useful especially in connection with
>> BFI's -view-block-freq-propagation-dags.  It helped me to track down the
>> bug that is being fixed in D24118.
>> 
>> While -view-block-freq-propagation-dags displays the high-level
>> information with static heuristics included (and block frequencies), the
>> new thing only shows the raw weight as presented by PGO without any of
>> the static estimates.  This helps to distinguished what has been
>> measured vs. estimated.
>> 
>> For the sample loop in D24118, -view-block-freq-propagation-dags=integer
>> looks like this:
>> 
>> https://reviews.llvm.org/F2381352
> 
> These links aren't visible without logging into phab, which seems a bit
> unfortunate. I filed llvm.org/PR30252 <http://llvm.org/PR30252>.

:(

> 
>> While with -view-cfg-only you can see the underlying branch weights:
>> 
>> https://reviews.llvm.org/F2392296 <https://reviews.llvm.org/F2392296>
>> 
>> Reviewers: dexonsmith, bogner, davidxl
> 
> When committing, please edit "Reviewers" to accurately reflect people
> who reviewed the patch, or just rip it out completely. The change does
> LGTM, but I find the way that phab/arc/whatever blindly lists people who
> happened to be CC'd as reviewers quite misleading.

Yeah, I found that confusing in the past as well but I guess I’ve gotten used to interpreting it the right way.  Unfortunately anybody using arcanist to manage Phab will have this problem.  Do you want to perhaps bring this is up on llvm-dev to try to get a consensus.  Or maybe this is something that can be adjusted on the server side by Manuel?

> 
>> Subscribers: llvm-commits
>> 
>> Differential Revision: https://reviews.llvm.org/D24144
>> 
>> Modified:
>>    llvm/trunk/include/llvm/Analysis/CFGPrinter.h
>> 
>> Modified: llvm/trunk/include/llvm/Analysis/CFGPrinter.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CFGPrinter.h?rev=280442&r1=280441&r2=280442&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/CFGPrinter.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/CFGPrinter.h Thu Sep  1 19:28:26 2016
>> @@ -118,6 +118,36 @@ struct DOTGraphTraits<const Function*> :
>>     }
>>     return "";
>>   }
>> +
>> +  /// Display the raw branch weights from PGO.
>> +  std::string getEdgeAttributes(const BasicBlock *Node, succ_const_iterator I,
>> +                                const Function *F) {
>> +    const TerminatorInst *TI = Node->getTerminator();
>> +    if (TI->getNumSuccessors() == 1)
>> +      return "";
>> +
>> +    MDNode *WeightsNode = TI->getMetadata(LLVMContext::MD_prof);
>> +    if (!WeightsNode)
>> +      return "";
>> +
>> +    MDString *MDName = cast<MDString>(WeightsNode->getOperand(0));
>> +    if (MDName->getString() != "branch_weights")
>> +      return "";
>> +
>> +    unsigned OpNo = I.getSuccessorIndex() + 1;
>> +    if (OpNo >= WeightsNode->getNumOperands())
>> +      return "";
>> +    ConstantInt *Weight =
>> +        mdconst::dyn_extract<ConstantInt>(WeightsNode->getOperand(OpNo));
>> +    if (!Weight)
>> +      return "";
>> +
>> +    // Append a 'W' to indicate that these are weights rather than actual
>> +    // profile
>> +    // count (due to scaling).
> 
> typo - this should say "Prepend a 'W'". The comment is also wrapped a
> little funny, probably from clang-format getting confused.

r280508.  Thanks!

Adam

> 
>> +    Twine Attrs = "label=\"W:" + Twine(Weight->getZExtValue()) + "\"";
>> +    return Attrs.str();
>> +  }
>> };
>> } // End llvm namespace
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160902/0e7b608d/attachment.html>


More information about the llvm-commits mailing list