Meinersbur wrote: How many cases/subclasses do you think will eventually be needed? From what I see it might be either shared memory or standard alloca. That is one additional member for OutlineInfo. This PR currently adds already two. I think only one subclass of CodeExtractor is necessary; it can receive a pointer to `OutlineInfo` and decide in its methods what kind of alloca it wants to create (Although now that you mention it, separate classes might be nicer).
A lambda is already syntactic sugar for a new (anonymopus) class. The additional data captured by it ist still there, but as members of that anonymous class. One may prefer that syntactic sugar over writing classes manually, but becomes problematic if it stops representing the logical structure of the program. I think the biggest problem with callbacks that are not limited to a local scope is that it is hard to know what is being called. How do you find all possible implementations of it if you need to fix change the semantics? What does the callback have to do or is allowed to do? This easily leads to a "devil's contract" where lambdas just do anything because it worked at some point, but the caller was not designed to allow this. Control flow is unpredictable turning a program into spaghetti code. Some say "Callbacks are the modern goto". In this case there is an obvious structure that point to a functional subclass: 1. Allocator and deallocator must be paired, you cannot combine them arbitrarily -> same class 2. There is a default implementation[^3] -> virtual method with default implementation in the base class 3. The callbacks are expected to do something, and the CodeExtractor stops working if they don't -> not a typical callback (informing another component that something has happened) 4. The callbacks are expected to do nothing else 5. OutlineInfo already contains the information the CodeExtractor is supposed to need[^2] 6. Using shared memory is a reusable feature, other constructs than parallel may want to use it without copy&pasting parallel's lamda. 7. Using shared memory is a feature that should be discoverable by programmers without looking into parallel's internal working. [^2]: That might indicate that OutlineInfo itself could be that derived class, but I wouldn't go that far. Composition is usually preferred over inheritance. [^3]: Rather checking whether the callback is nullptr, the ctor could assign a default callback function that contains the default alloca creation I think understandable and structured code is much more important than saving a few lines because lambdas are convenient. Unless we have a depenency inversion problem to solve, additional members to OutlineInfo and allocation implementations can always be added. https://github.com/llvm/llvm-project/pull/150925 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
