[MooreToCore] Support pow op

[fabianschuiki]

Add support for the moore.pow* ops in MooreToCore: https://chipsalliance.github.io/sv-tests-results/?v=circt_verilog+11.4.3+binary_op_pow

[maerhart]

We already have canonicalizers and folders that cover the simple cases here: https://github.com/llvm/circt/blob/337dad583965ea03ddcef75d9da2b5f10c23ec8d/lib/Dialect/Moore/MooreOps.cpp#L1131-L1197

The folders are automatically called during Dialect Conversion, the canonicalization aren't.

@fabianschuiki How should the a ** b case in the table be handled here? Since there's no pow op in Comb.

[fabianschuiki]

:+1: We could start out with a pretty dumb iterative approach? Maybe an scf.for that just goes from 0 to b and keeps multiplying a result by a? Pretty sure this almost never comes up in practice, and that would make it more of a performance issue than a correctness issue.

[Max-astro]

Hi @fabianschuiki, Following the iterative approach you mentioned, I’m trying to modify the PowUOp::canonicalize function to unroll the operator into an scf.for loop. However, I encountered the following error:

upow.v:6:9: error: failed to legalize operation 'scf.for'
    c = a ** b;
        ^
upow.v:6:9: note: see current operation: 
%24 = "scf.for"(%21, %22, %23, %1) ({
^bb0(%arg0: i32, %arg1: !moore.l32):
  %26 = "moore.mul"(%17, %arg1) : (!moore.l32, !moore.l32) -> !moore.l32
  "scf.yield"(%26) : (!moore.l32) -> ()
}) : (i32, i32, i32, !moore.l32) -> !moore.l32

I already added the scf::ForOp into the target.addDynamicallyLegalOp under the MooreToCore.cpp, but it didn’t have any effect. Could you provide some insight into what might be causing this issue? Thank you for your time and help!

[maerhart]

Hi @Max-astro, The canonicalize function is not called by the dialect conversion framework. You need to add a ConversionPattern to MooreToCore.cpp and add it to the pattern set. You can then implement your pow operator to scf.for transformation in that pattern. Unfortunately, canonicalization patterns and conversion patterns are not compatible, so you also need to replicate the existing canonicalization patterns functionality inside the conversion pattern.

[fabianschuiki]

It might be a good idea to do the scf.for conversion only in the conversion pattern of MooreToCore. Otherwise we would always convert moore.pow to scf.for immediately, which may not be what we want. (Other passes at the Moore dialect level might want to reason about the moore.pow op directly, e.g. for constant folding and the like.)

[Max-astro]

Thank you all for your suggestions @maerhart and @fabianschuiki . Your tips are very helpful and have given me some great ideas to try.