[PATCH] D140908: [MemProf] Context disambiguation cloning pass [patch 1a/3]

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 21 10:40:29 PDT 2023


tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp:1513
+  static std::string getNodeAttributes(NodeRef Node, GraphType) {
+    std::string AttributeString = (Twine("tooltip=\"") + getNodeId(Node) + " " +
+                                   getContextIds(Node->ContextIds) + "\"")
----------------
snehasish wrote:
> Maybe just declare AttributeString as Twine and change the return statement to `return AttributeString.str();` ?
> 
> Similar pattern of usage below too if choose to update this one.
Actually this didn't work here, since Twine's operator= is deleted, and getNodeAttributes requires conditional updates to the string. I was able to modify getEdgeAttribute below to use a single Twine, however. I also had to change getContextIds back to using std::string for similar reasons, but updated it to use Twine instead of std:to_string.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140908/new/

https://reviews.llvm.org/D140908



More information about the llvm-commits mailing list