asbirlea marked 2 inline comments as done.
asbirlea added a comment.
Thank you for all the comments.
================
Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:325
+ auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
+ return !empty(children<GraphDiffNodePair>({&GD, N}));
}
----------------
kuhar wrote:
> asbirlea wrote:
> > 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?
> Does allocating a GD have a big cost?
>
> I think you could get away with returning a temporary GD object that would
> die as soon as the expression evaluation ends -- should be no different than
> placing it on the stack just before the function call.
>
> Overall, I still don't understand why you try to avoid creating a wrapper
> function / struct that returns children, and call `childredn<...>(...)`
> directly. Either way, I being able to write:
> ```
> return !empty(children<GraphDiffNodePair>({getGD(BUI), N});
> ```
> or
> ```
> return !empty(getChildren<GraphDiffNodePair>(BUI, N));
> ```
> would definitely be concise and readable enough IMO.
> Does allocating a GD have a big cost?
AFAIU, dynamically allocating the object does, vs having a stack variable.
I'll do a follow-up cleanup attempt.
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
