search

llvm / circt

Circuit IR Compilers and Tools
https://circt.org
Other
1.66k stars 294 forks source link

System assembly Python API #805

Closed teqdruid closed 3 years ago

teqdruid commented 3 years ago

For ESI, I'd like to create a Python API to drive CIRCT so users can write python scripts to do system assembly. The general flow would be:

Most of these tasks are not ESI-specific. Should this API be a general CIRCT API, which just happens to have extra ESI support (in an ESI namespace) or should it be an ESI API/tool? This, I think, is just a matter of naming.

I mostly just wanted to check if this overlaps with anybody else's goals and perhaps write it more generally / collaborate on it.

mikeurbach commented 3 years ago

This is in line with my goals for the Python bindings (which I think you knew already), and generally fits with how I'd like to use ESI (still on my TODO list). One question about this bulleted list: is your intent to wrap it all up in one Python API, or would each bullet point have a Python API? To me, these could be several APIs that fit together. We could also have a pure-Python API that ties all of them together into one mega pass, if that is desirable, but I think some of these will be useful on their own.

Regarding naming: I think there are things here that should be part of a general CIRCT API, and some of the ESI things you mentioned should probably be in the ESI namespace.

For things that are useful on their own, like a Python API for running CIRCT passes, I would think something like from circt.passes import lower_to_rtl or something like that would make sense.

For things that are specific to ESI, I would expect from circt.esi import wrap_with_channel or something like that.

Just my two cents.

teqdruid commented 3 years ago

One question about this bulleted list: is your intent to wrap it all up in one Python API, or would each bullet point have a Python API?

I don't know what the practical difference would be at this point. (If CIRCT grows considerably, it may become unwieldy.) I'm thinking we just have one python API for all of circt, namespaced appropriately wherein we define "appropriately" on a case-by-case basis. Anything ESI specific should definitely go in a circt.esi namespace.

It sounds like we are in agreement.

mikeurbach commented 3 years ago

Sorry, I was being too vague, but yes we are in agreement. I sort of think of hypothetical circt.passes and circt.translations Python modules as different APIs, if that helps clarify my comments. Anyway, your wording is what I am in favor of: one python API for all of circt, namespaced appropriately.

teqdruid commented 3 years ago

I'm starting to implement the C++ support for this (there are several functions which are necessary for ESI). These functions are intended to be called from Python; however, I would like them to execute as part of the PR gate checks rather than just the nightly. I can think of two ways to accomplish this: 1) Add Python API tests to the PR gate build/test. 2) Write a C++ driver for testing purposes only.

Are there any other alternatives? If not, I kinda favor option 1. I could deal with option 2, but I would probably just add the tests to the integration test instead.

mikeurbach commented 3 years ago

I am not opposed to 1., but it probably means we will need to always build the PR gate with BUILD_SHARED_LIBS. Not sure if that is a problem for anyone... it may actually force some better habits as we develop.

For option 2., there is actually some precedent upstream in MLIR. Once you get past the setup and boilerplate, you basically write your test case in a nanopass. For example, here's what tests FunctionLike::eraseArguments: https://github.com/llvm/llvm-project/blob/c8cf8bc7ec3f8721953d4545812aaac5d21ee97a/mlir/test/lib/IR/TestFunc.cpp#L34. This is exercised in various ways through a regular filecheck test: https://github.com/llvm/llvm-project/blob/c8cf8bc7ec3f8721953d4545812aaac5d21ee97a/mlir/test/IR/test-func-erase-arg.mlir#L5.

Maybe there are other alternatives.

teqdruid commented 3 years ago

I am not opposed to 1., but it probably means we will need to always build the PR gate with BUILD_SHARED_LIBS. Not sure if that is a problem for anyone... it may actually force some better habits as we develop.

Actually, we already do for the 'clang release` build.

For option 2., there is actually some precedent upstream in MLIR.

There's also precedent in CIRCT -- the C API test. (Though it's not nearly a substantial as the one in MLIR that you point to.