Extract CFDFC from standard level

[yuxwang99]

No description provided.

[lucas-rami]

I just had a very quick look and will do a full review before tomorrow night. In the meantime, can you remove everything from the PR that is not just CFDFC extraction? I see that there is code related to declaration/definition of the buffer pass for example in there, which is not directly related to CFDFC extraction (it's the next PR you will make after we merge this). In the spirit of doing more atomic/managable PRs, only the code directly handling CFDFC extraction should be in this PR.

[lucas-rami]

I started to review and left a bunch of comments here and there but to me (as I commented above) it seems that the code entangles MILP solving and CFDFC extraction, whereas the goal was to only push things related to CFDFC extraction in this first step in order to keep the review manageable. For example, if I undestand buffer placement correctly, there shouldn't be any need for Gurobi at this stage since CFDFC extraction is essentially a graph processing step.

Anyway, I'm struggling to see where everything fits so maybe it's better to meet and discuss directly.

[lucas-rami]

As we said today, it looks like you could simplify your unit and channel structs (or even remove them entirely) with Operation* and Value, respectively. These are equivalent under the condition (that you can meaningfully make for this work) that every value is used exactly once (i.e., that forks and sinks have been materialized prior to the pass running). If the condition is not met, the pass/function should indicate a failure.

You can check the condition by using the following code snippet (funcOp is a handshake::FuncOp) at the beginning of the buffer pass / CFDFC extraction:

if (failed(verifyAllValuesHasOneUse(funcOp))) {
  funcOp.emitOpError() << "not all values are used exactly once";
  return failure(); // or do something that makes sense in the context
}

The verifyAllValuesHasOneUse function is defined in "circt/Dialect/Handshake/HandshakePasses.h".

[lucas-rami]

Given the amount of code related to buffer placement and the fact that we ultimately want to have it built conditionally based on whether Gurobi is installed, I think it would be good to group all related files in a subdirecory. For example, you could put HandshakePlaceBuffers.h, UtilsForExtractMG.h, and UtilsForPlaceBuffers.h in include/dynamatic/Transforms/BufferPlacement, and similarly for .cpp files in the lib folder.

Also, you can rename UtilsForExtractMG.[h|cpp] to ExtractMG.[h|cpp] to make it shorter, I think having UtilsFor in the name doesn't bring much in terms of clarity. As for UtilsForPlaceBuffers.[h|cpp], I think they should simply be merged into HandshakePlaceBuffers.[h|cpp] given that all these files are relatively small. Finally, all of the public functions (e.g., isEntryOp) which don't strictly need to be public at this point or in the near future should just be static functions defined in HandshakePlaceBuffers.cpp and without a corresponding declaration. The rationale is that, if there is no use case (as of now) for the function to be called outside of the pass, it shouldn't be public, especially if it's a super short function that is just a helper very specific to the pass.

[lucas-rami]

Left some comments, mostly cosmetic stuff. We need to setup conditional compilation depending on whether Gurobi is installed on the system. I may have a go at it myself on the branch.

[lucas-rami]

Just left a couple minor comments, after they are solved we can merge!