EPFL-LAP / dynamatic

DHLS (Dynamic High-Level Synthesis) compiler based on MLIR
Other
60 stars 18 forks source link

[Back Annotation] Back Annotation Ignoring Buffer Constraints #59

Closed kb29x37 closed 9 months ago

kb29x37 commented 9 months ago

Running the back-annotation flow with buffer constraints currently ignores some of the asserted constraints. This can result in buffers placed on edges of the circuit graph with a max tehb / oehb constraint set to 0. The input file and steps to reproduce afterward are for the FIR benchmark under integration-tests.

Input files:

Steps to reproduce: Simply run from the root of the project: ./tools/dynamatic/scripts/compile_custom.sh $(pwd) $(pwd)/integration-test/fir $(pwd)/integration-test/fir fir 0

Investigating the dot under: integration-test/fir/comp/fir.dot

There is buffer10 before fork5. However checking the constraints.json we can see:

{
      "operation-name": "fork5",
      "operand-idx": 0,
      "attribute-type": "bufProps",
      "attribute-data": {
          "minTrans": 0,
          "maxTrans": 0,
          "minOpaque": 0,
          "maxOpaque": 0,
          "inDelay": 0,
          "outDelay": 0,
          "delay": 0.0
      }
},

Which should prevent the placement of any buffers before fork5.

Thanks,

lucas-rami commented 9 months ago

Thanks for the report!

The issue was in fact that the constraints.json file was not respecting the keywords that our JSON format expects , which are slightly different from what was described on the issue that proposed the initial design of the back-annotation pass. The expected JSON format is described formally in the pass's in-source documentation. I attach the corrected constraints.json for reference, which when used seems to respect the per-channel constraints.

The pass was incorrectly "failing silently" when the attribute type was unknown, which explains why using "bufProps" instead of "buffering-properties" did not trigger a visible error and just caused the pass to ignore the annotation (explaining why buffering properties were not respected in the end). I have just fixed this. The original constraints.json file now fails with a nice error message :)