llvm / circt

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

[MooreToCore] Add conversion support for conditional and yield. #7162

Open angelzzzzz opened 2 weeks ago

angelzzzzz commented 2 weeks ago

Convert moore.conditional to comb.mux by moving the bodies of the true and false regions outside. It also remains doubt that it can not just evaluate one of the expressions depending on the situation, and will evaluate both. is it acceptable, or is there a more suitable way to lower conditional op?

fabianschuiki commented 2 weeks ago

One thing we could do is add the recursive side effect op interface to moore.conditional. That will allow you to ask the ConditionalOp whether any of its nested ops has side effects. If they do, you could lower into an scf.if { ... } else { ... } to preserve the conditional execution, and otherwise lower to comb.mux as you do right now.

mingzheTerapines commented 2 weeks ago

One thing we could do is add the recursive side effect op interface to moore.conditional. That will allow you to ask the ConditionalOp whether any of its nested ops has side effects. If they do, you could lower into an scf.if { ... } else { ... } to preserve the conditional execution, and otherwise lower to comb.mux as you do right now.

We found scf.if will be transformed to exportverilog finally, which is useless. But comb.mux could be transformed to llvmIR.

hailongSun2000 commented 2 weeks ago

The scf dialect can be handled by arcilator, it will be lowered into cf, then into llvm, if I remember correctly.

angelzzzzz commented 2 weeks ago

One thing we could do is add the recursive side effect op interface to moore.conditional. That will allow you to ask the ConditionalOp whether any of its nested ops has side effects. If they do, you could lower into an scf.if { ... } else { ... } to preserve the conditional execution, and otherwise lower to comb.mux as you do right now.

@fabianschuiki Considerate idea! We do need to handle the side effects. So is it acceptable to lower into an scf.if for any situation instead of converting to comb.mux? If we do this, should we handle scf.if and lower it to other dialects in MooreToCore?

hailongSun2000 commented 2 weeks ago

As above mentioned, should we tag the scf dialect as legalization?

fabianschuiki commented 2 weeks ago

Marking scf.if as legal in MooreToCore makes a lot of sense! That will allow us to generate scf.if where we need it 😃.

I think it would make sense to lower to comb.mux whenever possible, because it's explicitly side-effect free. And in the cases where there are side effects it would be great to generate an op like scf.if which explicitly can have side-effects.