[llvm] [MemProf] Remove empty edges once after cloning (PR #85320)

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 26 11:09:18 PDT 2024


================
@@ -306,7 +306,7 @@ class CallsiteContextGraph {
     // True if this node was effectively removed from the graph, in which case
     // its context id set, caller edges, and callee edges should all be empty.
     bool isRemoved() const {
-      assert(ContextIds.empty() ==
+      assert(!ContextIds.empty() ||
----------------
teresajohnson wrote:

The original assert was wrong, which showed up in testing after the reorganization here. The case that was wrong was when we were left with a single (non-removed) node in the graph, which would have non-empty context ids, but no callee or caller edges. I played around with restructuring the assertion but didn't come up with anything that looked clearer to me. E.g. here's another structure but it feels more convoluted to me personally:

```
assert(!(ContextIds.empty() &&
               (!CalleeEdges.empty() || !CallerEdges.empty()));
```

It's not inconsistent with the comment (it is still true that all 3 should be empty if it was removed) but I've added new comments to explain the case described above. Let me know if you have a preference of what to do for the assert structure.

https://github.com/llvm/llvm-project/pull/85320


More information about the llvm-commits mailing list