[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 7 16:55:04 PDT 2020


asbirlea marked 4 inline comments as done.
asbirlea added inline comments.


================
Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:325
+    auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
+    return !empty(children<GraphDiffNodePair>({&GD, N}));
   }
----------------
kuhar wrote:
> This pattern repeats a few times. Could it be pushed into a helper function to get something like this?
> 
> ```
> return !empty(children<GraphDiffNodePair>(GetGraphDiffNodePair(BUI, N)));
> ```
> 
My dilemma here is how to not allocate a GraphDiffT instance. There are 4 cases where the pattern is inside a static method and once in a class method. I used a stack var for the 4 cases and a unique_ptr for the class method.

Sure, I can do:
```
static auto getGDNPair(BUI, EmptyGD, N) {
return std::make_pair<>(BUI ? BUI->PreViewCFG : EmptyGD, N);
}
{
...
GraphDiffT EmptyGD;
return !empty(children<GraphDiffNodePair>(getGDNPair(BUI, &EmptyGD, N)));
}
```
But it felt like moving one line at the callsite to a oneline helper is not making things much better.


I was hoping for something more along the lines of:
```
return !empty(children<GraphDiffNodePair>({getGD(BUI), N});
```
Or, ensure BUI is always non-null, and simplify to:
```
return !empty(children<GraphDiffNodePair>({BUI->PreViewCFG, N});
```


Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77341





More information about the llvm-commits mailing list