asbirlea added inline comments.
================ Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:185 + std::make_pair(GDTmp, BB); + for (auto &Pair : children<GraphDiffBBPair>(GDNodePair)) { + const NodePtr Succ = Pair.second; ---------------- kuhar wrote: > Won't children infer the template parameter based on the passes value? I > don't get why the template argument type is a pair where the second argument > is a directed nodeptr, but the runtime value is always a plain nodeptr. > Couldn't the second pair type also be directed for `GDNodePair`? The directed nodeptr is the equivalent of a bool inside GraphDiff translating to what kind of children do you want; the second pair argument bears no info of that kind, only the actual NodePtr for which we desire the children. ================ Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:186 + for (auto &Pair : children<GraphDiffBBPair>(GDNodePair)) { + const NodePtr Succ = Pair.second; const auto SIT = NodeToInfo.find(Succ); ---------------- kuhar wrote: > kuhar wrote: > > Could this new code be a hoisted into a helper function? > Or alternatively, could the old `ChildrenGetter` be implemented with these 5 > magic lines? I think it looks much cleaner after the latest update, let me know what you think. ================ Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:1543 + // FIXME: Updated to use the PreViewCFG and behave the same as until now. + // This behavior is however incorrect; this actually needs the PostViewCFG. + GraphDiff<typename DomTreeT::NodePtr, DomTreeT::IsPostDominator> PreViewCFG( ---------------- kuhar wrote: > Does this care about the direction (pre- or post-) at all, or does it need > some CFG view? Why is pre-view incorrect? The purpose of this method was to update the DT assuming an additional list of updates (the argument given). In practice, the BUI argument is ignored in the `CalculateFromScratch` call below. Hence we need the additional changes to support this kind of updates inside `CalculateFromScratch` and the other methods. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77341/new/ https://reviews.llvm.org/D77341 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits