[hls-fuzzer] Add cross type system transfer functions - #1023
Conversation
5e4f1da to
1e7cc19
Compare
Prior to this PR, every type system in a conjunction operated completely independent of other present type systems. This means that it was wholly responsible for context calculations based on its transfer functions. However, some type systems are explicitly designed as "action" type systems, that restrict program generation in some interesting way and do so based on some initial value in an input context. The 'OptionalTypeSystem' is e.g. one such case that allows only applying a sub-typesystem to some subset of the program. To enable the use case of "analysis" type systems that then cause an action in an "action" type system, this PR adds the concept of cross transfer functions. These are special transfer functions that can be used in subclasses of 'ConjunctionTypeSystemBase' and allows creating a transfer function that depends on another type system's context when calculating another's. Specific use-case for the future is that for a high II type system, we have one type system recognizing an innermost loop which then enables the generation of high latency reccurrences within innermost loops. This way neither type systems need to know of each other, only the conjuncting type system wires them up together.
94f1272 to
b12cf3b
Compare
Jiahui17
left a comment
There was a problem hiding this comment.
started looking at this super complicated one... some clarification questions:
| /// given type systems invariants. | ||
| template <std::size_t subElement, typename SubTypeSystem, | ||
| typename... dependencies, typename TransferFns, typename F> | ||
| static void crossTransferFns(TransferFns &transferFnArray, F &&f) { |
There was a problem hiding this comment.
Is f the one that crosses the results from different subtypesystem and does transferFnArray contain the transferFns of the subtypesystem?
If yes I suggest giving more descriptive and different names to them
There was a problem hiding this comment.
transferFns must contain the transfer funciton of the conjunction type system (i.e. whatever is returned by Base::get*TransferFn). f is the map that implements the crossing yes
There was a problem hiding this comment.
could you rename f to something like "crossingFunc"?
| // Self-references in dependencies are not legal in 'wrap' as the resulting | ||
| // node would depend on itself and cause a cycle! | ||
| // For that reason we filter them out here and reinsert them later. | ||
| using CalcFilter = FilterDeps<subElement, dependencies...>; |
There was a problem hiding this comment.
This is basically a utility struct that given a list of dependencies (i.e. Dep<TypeSystemIndex, index>) gets rid of any instances of Dep<..., subElement>.
This is needed since wrap doesn't handle self-dependencies (and shouldn't IMO). The filtered list of Deps is the Tuple below. The match function additionally tells us whether the given index originally referred to a Dep<TypeSystemIndex, index> and is used later.
I've added a comment
| // Filtered 'Dep' instances without 'subElement'. | ||
| using Tuple = typename CalcFilter::value; | ||
|
|
||
| auto &transferFn = std::get<subElement>(transferFnArray); |
There was a problem hiding this comment.
explain what this one is (the original transferFn before crossing?
There was a problem hiding this comment.
Exactly! I've added a comment
| .template wrap<Context, | ||
| std::decay_t<decltype(newDeps)>::index...>( | ||
| [f = std::forward<F>(f)]( | ||
| llvm::function_ref<Context()> wrapped, |
There was a problem hiding this comment.
the name wrapped is too generic here (could be something like transferFnWithCrossing
| std::get<typename SubTypeSystem::Context>(context) = | ||
| std::apply( | ||
| f, | ||
| // Finally, return the context of just the | ||
| // requested type systems. |
There was a problem hiding this comment.
What if we need to modify the context of multiple type systems?
There was a problem hiding this comment.
One can call crossTransferFns multiple times with different target type systems without issues, I've added it to the method description.
| std::forward<CrossingFunc>(crossingFunc)]( | ||
| llvm::function_ref<Context()> originalTransferFn, | ||
| const auto &...deps) { | ||
| // Context prior to the crossing. |
There was a problem hiding this comment.
Context computed by the transferFns, before crossing
| llvm::function_ref<Context()> originalTransferFn, | ||
| const auto &...deps) { | ||
| // Context prior to the crossing. | ||
| Context context = originalTransferFn(); |
There was a problem hiding this comment.
context -> contextBeforeCrossing
I find it quite confusing that we need to work with both
- the concrete values of the context
- the template parameters
But it is pretty hard to tell which one is which (e.g., is a variable named context a concrete value or a type calculated from the template parameter?
Is it possible to name the variables differently to make it more explicit?
| /// | ||
| /// This method makes it possible for the class implementing the conjunction | ||
| /// to implement logic that modifies the input context of 'subElement' | ||
| /// of an 'ASTNode' (deduced from its transfer functions). |
There was a problem hiding this comment.
/// to implement logic that modifies the input context (computed by the original TransferFns) of 'subElement'
| /// to implement logic that modifies the input context of 'subElement' | ||
| /// of an 'ASTNode' (deduced from its transfer functions). | ||
| /// | ||
| /// One call crosses exactly one sub element ('subElement') of exactly one sub |
| // First remove the 'ASTNode's from the argument list. | ||
| // This is now equal to just the context's without | ||
| // the self references. | ||
| auto contextsOnly = mapTuplesInto( |
There was a problem hiding this comment.
for instance, here, this variable is called contextsOnly (c.f., context) but it is a tuple of templated struct type (compared to the context which is an actual value
| /// Returns an 'OpaqueTransferFn' over 'TypingContext' that runs 'f' in place | ||
| /// of 'this', handing 'this' to it so that it can run it and adjust or | ||
| /// reinterpret what it computed. |
There was a problem hiding this comment.
What is 'this'?
Still not super sure why we need this
| if constexpr (std::is_same_v<Sentinel, T>) | ||
| return std::forward_as_tuple(context); | ||
| else | ||
| return std::forward_as_tuple(context, arg); |
There was a problem hiding this comment.
don't we need only the context values here? Why are we still forwarding Deps (typesystem + index)?
Prior to this PR, every type system in a conjunction operated completely independent of other present type systems. This means that it was wholly responsible for context calculations based on its transfer functions.
However, some type systems are explicitly designed as "action" type systems, that restrict program generation in some interesting way and do so based on some initial value in an input context. The 'OptionalTypeSystem' is e.g. one such case that allows only applying a sub-typesystem to some subset of the program.
To enable the use case of "analysis" type systems that then cause an action in an "action" type system, this PR adds the concept of cross transfer functions.
These are special transfer functions that can be used in subclasses of 'ConjunctionTypeSystemBase' and allows creating a transfer function that depends on another type system's context when calculating another's.
Specific use-case for the future is that for a high II type system, we have one type system recognizing an innermost loop which then enables the generation of high latency reccurrences within innermost loops. This way neither type systems need to know of each other, only the conjuncting type system wires them up together.