[PATCH] D114204: [CSSPGO] Sorting nodes in a cycle of profiled call graph.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/ProfiledCallGraph.cpp:138
+  for (auto *Node : InputNodes)
+    (void)NodeInfoMap[Node].Group;
+
----------------
wenlei wrote:
> hoy wrote:
> > wenlei wrote:
> > > hoy wrote:
> > > > wenlei wrote:
> > > > > nit: make this an explicit insert for readability
> > > > This is specifically used to construct a `NodeInfo` object in place. An insert operation will involve a copy construction which invalidate the initial value of the `Group` field which should be `this`. An emplace operation may be able to construct the object in place but I haven't figured out how to pass a none parameter into to.
> > > > 
> > > I thought `emplace()` without a parameter should just work, no? but yeah, I meant emplace.
> > No, neither `NodeInfoMap.emplace(Node);` nor `NodeInfoMap.emplace(Node, {});` works.
> I think what you need is `NodeInfoMap.emplace(Node, NodeInfo())`. See https://godbolt.org/z/Wrx61rYcY.
That’ll construct a temp object whose `Groups` points to itself. The temp object will be used to copy construct the final object whose `Groups` will point to the temp object which isn’t what we want. Also this still cannot avoid a copy construction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114204