[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