privacy-scaling-explorations / chiquito

DSL for Halo2 circuits
https://docs.pecadorplonkish.xyz/
186 stars 39 forks source link

Implement compilation to new SBPIR with multiple machines #288

Closed alxkzmn closed 1 month ago

alxkzmn commented 1 month ago

These changes do not touch the interpreter. The legacy code was already returning separate machine setups in case of multiple machines, so the changes take advantage of that. The files with _legacy suffix contain unchanged legacy code.

leolara commented 1 month ago

So, I didn't think how was this going to interact with the DSL, so I didn't tell you about. If you have a doubt you can ask me before.

The current dsl is going to be deprecated, for a rust DSL that can read fragments of chiquito source code.

Hence, the best option for this is that the new compiler doesn't use the DSL. Manipulate the SBPIR structure directly, even if it is more code in the compiler. If it makes sense some mutable methods could be added to the SBPIR if necessary.

alxkzmn commented 1 month ago

Could we spec it out a bit more? I understand this as removing any "dsl" imports from the new compiler and implementing it without them, is this correct?

leolara commented 1 month ago

@alxkzmn yes the new compiler should not depend on this DSL, that is using as an API more than as a DSL.

We should implement in the compiler what we require from the DSL. The legacy compiler was easier to implement with the DSL, but it is not the best way to go

alxkzmn commented 1 month ago

Here are the updated specs:

Scope

The scope of the changes is the refactoring of the way the compiler creates a new SBPIR. Specifically, removing any code that depends on the dsl module.

Implementation

leolara commented 1 month ago

Yes.

So, to give you some context we built the DSL with lambdas and ctx to make it look more like a DSL and more readable. But it makes the code of the DSL more complicated.

So, if you think it is too much facilities that are useful from the DSL, you could transfer it to a SBPIRBuilder (and perhaps a series of other "builder" structs), where the API is normal and not based on lambdas and ctx.

Do you think we need this?

alxkzmn commented 1 month ago

I think I may borrow some code that is useful but I agree that we don't need a "lambda" anymore and generally don't need the APi-like syntax.

leolara commented 1 month ago

@alxkzmn some tests that are in legacy are not in the new compiler. A part from that approved