[hls-fuzzer] Introduce context dependency API

[zero9178]

Prior to this PR the order in which contexts were derived was hardcoded: Every AST element forwarded its context to all its child elements (with possibly some derivation happening). This works well for many type systems, specifically ones that mainly deal with expressions but hits limits when dealing with side-effecting statements. Examples of things that do not work with this system:

• We cannot constrain the indexing expression of an array assignment or array indexing based on the array parameter since this requires deriving the context of the expression from the array parameter
• We cannot forward data from one statement to another (either forwards or backwards), since these are sibling elements.

This PR therefore implements a new mechanism that is a strict superset of functionality from the existing check* methods by enabling every subelement to specify which subelements it depends on. The generator then performs a topological sort to generate subelements in some order satisfying the dependencies between contexts.

As an example use, the BitwidthTypeSystem can now generate inbounds expressions by restricting the bitwidth of the indexing expression to match the dimensions of the array parameter.

The API is currently only implemented for BinaryExpression and ArrayReadExpression but the goal is the remove the old check* API entirely. This will be done in a follow up PR

This is an alternative to #837 which implements the same functionality but in a less powerful and more composable fashion. Specifically, multiple type systems can still be combined into one as long as their dependencies do not create a cycle.

Note: For formal geeks, this implements Attribute grammars but every AST node currently has the same set of symbols

[Jiahui17]

I got quite lost in this PR haha

    return DependencyArray<ast::ArrayReadExpression>{
        copyFromParent<ast::ArrayReadExpression>(),
        Dependency<ast::ArrayReadExpression>([] {
          return DynamaticTypingContext{
              DynamaticTypingContext::IntegerRequired};
        }),
        copyFromParent<ast::ArrayReadExpression>()};

How do i read this? Does this dependency array specify any order?

[zero9178]

I got quite lost in this PR haha

    return DependencyArray<ast::ArrayReadExpression>{
        copyFromParent<ast::ArrayReadExpression>(),
        Dependency<ast::ArrayReadExpression>([] {
          return DynamaticTypingContext{
              DynamaticTypingContext::IntegerRequired};
        }),
        copyFromParent<ast::ArrayReadExpression>()};

How do i read this? Does this dependency array specify any order?

Yes! Every position in the array corresponds to the subelement we are calculating the context for.
In the case of an ArrayReadExpression, index 0 corresponds to the ast::ArrayParameter. copyFromParent<ast::ArrayReadExpression>(), returns a Dependency instance that just forwards the context that is used for ast::ArrayReadExpression (i.e. input context).
Index 1 is the indexing expression and here we don't depend on anything and just return IntegerRequired.

The very last position in a DependencyArray is special and represents an "output context", i.e. what the subelement returns back to the parent. This is admittedly a bit of foreshadowing in that it doesn't fully work yet (input context == output context for now), but will be useful for statements and other things.

[Jiahui17]

Oh I meant how this dependencyarray specifies the order of AST node generation, for instance:

Specification 1:
1. Generate RHS
2. Generate Index
3. Generate array name

Specification 2 (notice that the order is different now):
1. Generate array name
2. Generate RHS
3. Generate Index

[zero9178]

Oh I meant how this dependencyarray specifies the order of AST node generation, for instance:

Specification 1:
1. Generate RHS
2. Generate Index
3. Generate array name

Specification 2 (notice that the order is different now):
1. Generate array name
2. Generate RHS
3. Generate Index

The order of AST node generation is implicit through what input dependencies the Dependency instances have.

For example, for your first specification, one possible DependencyArray that would force that order is as follows:

return DependencyArray<ast::ArrayAssignmentStatement>{
     // Depends on the index (index 1 in the SubElements tuple).
    /*arrayName=*/Dependency<ast::ArrayAssignmentStatement, 1>([](const Context& context, const ast::Expression& index) {
        return ...;
    }),
    // Depends on the value (index 2 in the SubElements tuple).
    /*index=*/Dependency<ast::ArrayAssignmentStatement, 2>(...),
    // Depends on nothing.
    /*value=*/Dependency<ast::ArrayAssignmentStatement>(...),
    ...
};

One can also specify more than one index. E.g. the array name could also depend on both the value and index and the ordering would be unchanged. The framework performs a topological sort according to these dependencies to generate the AST nodes in that order.

An abbreviated example for your second specification would e.g. be:

return {
     // Only depends on input context.
     Dependency<ast::ArrayAssignmentStatement, PARENT_CONTEXT>(...),
     // Requires array name + index.
     Dependency<ast::ArrayAssignmentStatement, 0, 1>(...),
     // Requires only the array name.
     Dependency<ast::ArrayAssignmentStatement, 0>(...),
     ...
};

[Jiahui17]

I am a bit worried now that configuring it is more complicated than hardcoding the order...

[zero9178]

I am a bit worried now that configuring it is more complicated than hardcoding the order...

This approach does have many advantages and I don't think is more complicated than hardcoding the order. Specifically:

• We don't actually care about the order in the first place, just about dependencies for calculating contexts. This is more explicit about that. The specific order of AST generation is a pure implementation detail.
• This approach is composable. If we hardcode orders like in the other PR, then you cannot use two type systems at the same time: Both type systems specify some fixed order which is much stricter than the partial order that they actually care about. The approach here only specifies partial orders between subelements which can simply be unioned without issues as long as it doesn't result in a cycle (which is detectable as well)

Its also less powerful in that it doesn't allow nor make the type system care about generating AST nodes.

[Jiahui17]

Ok, I got this now, i am convinced that this is the way to go about it.

As far as i understood, we had:

DependencyArray:

• (Requirement 1) A collection of transfer functions called "Dependency"s (the
  syntax can be statically derived from the typesystem traits)

Dependency:

• (Requirement 2) Specifies the input argument (can be 1. the input context, 2. an output context of another output, 3. nothing) of the transfer function.
• (Requirement 3) Specifies the transfer function from the AST node input to output

I like the separation for requirement (1,) and (2, 3,) now, maybe we could try to improve the readability like this:

return {
     // Only depends on input context.
     Dependency<ast::ArrayAssignmentStatement, PARENT_CONTEXT>(... /* TF functions */),
     // Requires array name + index.
     Dependency<ast::ArrayAssignmentStatement, ARRAY_NAME, INDEX>(... /* TF functions */),
     // Requires only the array name.
     Dependency<ast::ArrayAssignmentStatement, ARRAY_NAME>(... /* TF functions */),
     ...
};

---

I like that the template parameters specify the signature of the transfer function:

Dependency<
  /* Used to infer return type */ ast::ArrayAssignmentStatement,
  /* Used to infer the types of the arguments*/ PARENT_CONTEXT, ....>( /* TF function */ );

Question: Is there a way to communicate this design consideration with the reader more explicitly?

---

Question: Is there a better name for Dependency?

[zero9178]

return {
     // Only depends on input context.
     Dependency<ast::ArrayAssignmentStatement, PARENT_CONTEXT>(... /* TF functions */),
     // Requires array name + index.
     Dependency<ast::ArrayAssignmentStatement, ARRAY_NAME, INDEX>(... /* TF functions */),
     // Requires only the array name.
     Dependency<ast::ArrayAssignmentStatement, ARRAY_NAME>(... /* TF functions */),
     ...
};

I like that the template parameters specify the signature of the transfer function:

Absolutely, we could just add the indices to e.g. ast::ArrayAssignmentStatement, then it'd read like ast::ArrayAssignmentStatement::ARRAY_NAME e.g.

Dependency<
  /* Used to infer return type */ ast::ArrayAssignmentStatement,
  /* Used to infer the types of the arguments*/ PARENT_CONTEXT, ....>( /* TF function */ );

Question: Is there a way to communicate this design consideration with the reader more explicitly?

The return type of the transfer function is always a context. The first argument ast::ArrayAssignmentStatement is more or less used in conjunction with the indices to infer the arguments.

I tried to add in the documentation of Dependency how the signature is derived from the input dependencies template argument.

Question: Is there a better name for Dependency?

I agree actually. It feels more imperative than declarative. The key-service is actually the transfer function, the generation order is a side-effect.
What do you think about something like TransferMethod? Could techncically also use TransferFunction or TransferFn but function might just be a bit overloaded. Method to me sounds a bit like it does more than just pure functional computation, but also includes the dependency part while not putting it in the foreground,

[Jiahui17]

Absolutely, we could just add the indices to e.g. ast::ArrayAssignmentStatement, then it'd read like ast::ArrayAssignmentStatement::ARRAY_NAME e.g.

Makes sense

---

The return type of the transfer function is always a context. The first argument ast::ArrayAssignmentStatement is more or less used in conjunction with the indices to infer the arguments.

I tried to add in the documentation of Dependency how the signature is derived from the input dependencies template argument.

BTW, this reminds me of the std::function: std::function<RetTy(Arg1Ty, Arg2Ty, ...)> f = []() {...} (notice that syntax separates the return type and the argument types).

---

What do you think about something like TransferMethod? Could techncically also use TransferFunction or TransferFn but function might just be a bit overloaded. Method to me sounds a bit like it does more than just pure functional computation, but also includes the dependency part while not putting it in the foreground,

I think either ContextTransferFn or TransferFn is okay because you already came up with a separate name for the pure computation part (ContextComputationFn)

[Jiahui17]

So it is not debating whether it should be called Method or Function but Computation vs. Transfer

[Jiahui17]

BTW what is the role of the check* methods now? Feels like there is some overlapping between the getDependencies and the check* now

[zero9178]

TransferFn sounds good to me!

BTW what is the role of the check* methods now? Feels like there is some overlapping between the getDependencies and the check* now

It is very much overlapping yes. check* methods are a subset of functionality: They act as if all TransferFn instances had only a PARENT_DEPENDENCY and share one single transfer method.

I will remove the check* methods in a subsequent PR. I didn't want to it yet in this one for the ease of review and since it is already large enough

[Jiahui17]

Maybe let's set up a convention for the code here:

// arrayName[index] = rhs;
auto arrayNameTransferFn = ...
auto arrayIndexTransferFn = ...
auto rhsTransferFn = ...

return {arrayNameTransferFn, arrayIndexTransferFn, rhsTransferFn};

[zero9178]

I decided against doing this since I dislike how it disconnects usage site with definition.
Specifically, the position within the array is very meaningful since it corresponds to specific AST subelements. The variable approach also allows the error of mismatching the order of declarations of the variables and order of using those variables in the variable.

I instead opted to just annotate the parameters using the /*name=*/ convention

[Jiahui17]

Split

auto contextTransferFn = ...;
auto lhsGenFn = ...;
auto rhsGenFn = ...'
auto expressionGenFn = ...;

return generateWithDependencies<ast::BinaryExpression>(context, contextTransferFn, lhsGenFn, rhsGenFn, expressionGenFn);

[zero9178]

See #874 (comment)

I added argument comments instead.

[Jiahui17]

Count? What does it mean

[zero9178]

These are the number of incoming edges for a given node i. The algorithm primaril works on outgoing edges since scheduling a node removes all outgoing edges from that node from the graph, but after doing so we need to check whether the number of incoming edges of a node is now empty/0 to check whether it can now be scheduled.

I've added comments to make this more explicit

[zero9178]

I've also decided to move the SubElements member into the ASTNode class instead. The idea is that what SubElements an ASTNode consists of is more a trait of the the AST node and not really something that the typesystem augments onto the AST node. This also means that we can get rid of TypeSystemTraits in the future entirely

[Jiahui17]

yeah it does feel like a property of an ast node

[Jiahui17]

Redundancy in the SubElements tuple and the actual subelement class members?

[zero9178]

Redundancy in the SubElements tuple and the actual subelement class members?

Kind of but kind of not. One could calculate the index 0 at constexpr time by searching for ast::ArrayParameter in the std::tuple but you e.g. cannot do this properly when the tuple contains a type T multiple time (e.g. lhs and rhs in a BinaryExpression are both ast::Expression).
So neither of the two members really imply the other and you need both. If they mismatch/disagree you should be able to catch the type errors as soon as you use them in TransferFn since the ASTNode parameters of the transfer functions would mismatch expectations.

There is probably a better way to do this, but I think this is a good start

[Jiahui17]

Okay

Another thing is the OpaqueTransferFn: what is the problem that this class tries to solve? Like a convience placeholder type that works for all possible template values of TransferFn?

[zero9178]

Okay

Another thing is the OpaqueTransferFn: what is the problem that this class tries to solve? Like a convience placeholder type that works for all possible template values of TransferFn?

Correct yes! Kind of similar to OpaqueContext is to other contexts