[PATCH] D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG.

Bardia via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 9 14:14:45 PDT 2019


bmahjour marked 2 inline comments as done.
bmahjour added a comment.

In D64088#1574555 <https://reviews.llvm.org/D64088#1574555>, @Meinersbur wrote:

> To reduce the number of allocations, have you thought about making EdgeList contain the edges objects instead of pointers? The edges would have to be copied/moved into the list and edges could not be compared by identity. Is this semantic needed/are edge objects large?
>
> Since the edge class does not contain the source node, the same edge object could be put into multiple outgoing edges lists. Is this supported?


I think we would be better off using pointers in this case, because of the following reasons:

1. Using pointers gives the clients freedom to use polymorphic behavior.
2. Using pointers avoids the copy and moves you mentioned. Given that this class is intended to be extended by client code, it's probably better not to assume that the copy/moves will always be cheap.
3. Using pointers allow us to do the edge optimization you mentioned. This is currently not being done, it's something we can look into if memory usage becomes an issue.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64088





More information about the llvm-commits mailing list