bmahjour added inline comments.
================
Comment at: llvm/include/llvm/Analysis/DDG.h:480
+ return OS.str();
+ unsigned count = 0;
+ for (auto &D : Deps) {
----------------
Meinersbur wrote:
> [suggestion] Instead of count, you can use `llvm::enumerate`. I think it's
> nicer to check at for "not the first" at the beginning instead of the last
> element at the end.
>
> There also is an `interleave` function in STLExtras.h for uses cases like
> this.
Interesting! Thanks for pointing out those utilities! I'll use interleave.
================
Comment at: llvm/include/llvm/Analysis/DDGPrinter.h:45
+ assert(G && "expected a valid pointer to the graph.");
+ Graph = G;
+ return "DDG for '" + std::string(G->getName()) + "'";
----------------
Meinersbur wrote:
> Why does a getter set a field value?
Good question :)
When printing pi-blocks we would like to show the member nodes being enclosed
by the pi-node. However, since the member nodes are also part of the graph,
they will get visited during the walk and get dumped into the resulting dot
file. To solve this, I'm trying to hide the member nodes from the walk (via
`isNodeHidden()`) when they get visited and then print them as part of printing
the pi-block node.
The problem I encountered was that, `isNodeHidden()` only takes a graph node as
a parameter, but for me to know whether a node belongs to a pi-block I need to
get access to the graph that contains the nodes. To get around this, I'm
caching a pointer to the graph when the `getGraphName` gets called and use it
when `isNodeHidden` is invoked....yikes!
A cleaner solution (and one that may have other uses in the future) is to make
`isNodeHidden()` take a graph object as argument (similar to `getNodeLabel()`,
and `getGraphName()`). I'll update the patch to include that change.
================
Comment at: llvm/lib/Analysis/DDGPrinter.cpp:109
+ if (isa<SimpleDDGNode>(Node))
+ for (auto *II : static_cast<const SimpleDDGNode
*>(Node)->getInstructions())
+ OS << *II << "\n";
----------------
Meinersbur wrote:
> Does this const_cast exist to add const qualifier?
No. The static_cast is used because the `getInstructions()` is a member
function of `SimpleDDGNode` but not `DDGNode`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90159/new/
https://reviews.llvm.org/D90159
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
