[MooreToCore] Lower exponentiation to math.ipowi (PowUOpConversion)

[liamslj13]

This pull request addresses part of issue #8476, and is related to PR #8574. Similar to the signed integer conversion, I have removed the SCF dialect for-loop logic and replaced it directly with math.ipowi from MLIR's Math dialect.

Initially, it was suggested to zero extend the inputs (would this require mlir.llvm dialect? More specifically llvm.zext), and to ensure this conversion is secure I can reevaluate my changes to reflect this suggestion. However, I submit this PR now due to tests currently passing in this state.

[fabianschuiki]

Really cool, thanks for finishing #8476 🥳

As you pointed out, I think for this unsigned case you will need to add a zero bit at the top of the operands, and then remove that again later.

Consider an 8-bit moore.powu operation with two integers:

%0 = moore.constant 200 : i8
%1 = moore.constant 201 : i8
%2 = moore.powu %0, %1 : i8

The math.ipowi operation is signed, which means it interprets its operands as signed values and produces a signed result. This is problematic with the constants 200 and 201, which are valid 8-bit unsigned values, but can't be represented as 8-bit signed values. Consider their bit patterns:

200 = 0b11001000  // unsigned interpretation
201 = 0b11001001  // unsigned interpretation

When you interpret these numbers as signed values, they look like negative numbers because they have the top-most bit set:

0b11001000 = -56  // signed interpretation
0b11001001 = -55  // signed interpretation

Unsigned 8-bit values cover the range [0,255], while signed 8-bit values cover the range [-128,127]. So when you pass an unsigned 8-bit value to a function that expects a signed 8-bit value, you'll get overflows where the function misinterprets the upper half of all possible unsigned values as a negative number.

An easy workaround for this issue is to add zero-extend the unsigned numbers by a single bit. In the example above, this will turn the 8-bit values into 9-bit values, where all possible 8-bit unsigned values are interpreted correctly if you represent them as 9-bit signed values:

// 9-bit extension
200 = 0b011001000  // unsigned interpretation
201 = 0b011001001  // unsigned interpretation
0b011001000 = 200  // signed interpretation
0b011001001 = 201  // signed interpretation

You can do this with comb.concat to zero-extend, and comb.extract to truncate afterwards:

func.func @example(%arg0: i8, %arg1: i8) -> i8 {
  %false = hw.constant false  // i1 value 0
  %0 = comb.concat %false, %arg0 : i1, i8  // zext %arg0 to i9
  %1 = comb.concat %false, %arg1 : i1, i8  // zext %arg1 to i9
  %2 = math.ipowi %0, %1 : i9
  %3 = comb.extract %2 from 0 : (i9) -> i8
  return %3 : i8
}

In code this probably looks something like this:

Type resultType = ...;
auto zero = rewriter.create<hw::ConstantOp>(loc, 1, 0);
auto lhs = rewriter.create<comb::ConcatOp>(loc, zero, adaptor.getLhs());
auto rhs = rewriter.create<comb::ConcatOp>(loc, zero, adaptor.getRhs());
auto pow = rewriter.create<math::IPowIOp>(loc, lhs, rhs);
rewriter.replaceOpWithNewOp<comb::ExtractOp>(op, resultType, 0);

[liamslj13]

Thank you for the thorough explanation! This makes things quite a bit clearer, and I was able to get some work done on it today. I had some issues with types but after looking through some docs and the previous implementation of PowSOpConversion I was able to scrap together some code which I will push shortly.

Just a question or two I have about some of the code that I wasn't fully able to understand! In lines

auto lhs = rewriter.create<comb::ConcatOp>(loc, zero, adaptor.getLhs());
auto rhs = rewriter.create<comb::ConcatOp>(loc, zero, adaptor.getRhs());

does comb::ConcatOp concatenate to the left or right side of the unsigned number (this might not actually matter)? Also would using a different dialect such as mlir::llvm not be viable here with its built in mlir::llvm::zext function? This might be overkill or add some unneeded bulk, but I will try anything that could make this work nicely 👍

Just maybe another small question as well; are both input integers guaranteed to be of the same type width, or would you recommend I look for length of the bigger one and convert the other side to the same width?

Anyways, if these changes look good to you then I can review the test in basic.mlir and try to learn more about the IR syntax as a whole. The current tests will not pass so I will work on writing some that do. Sorry about the indentation issue as well, should be fixed now.

[fabianschuiki]

Just a question or two I have about some of the code that I wasn't fully able to understand! In lines [...] does comb::ConcatOp concatenate to the left or right side of the unsigned number (this might not actually matter)?

The comb.concat operands go from most significant bit to least significant bit. So the first operands will land at the highest bits of the result (left-most if you write the number down), and the last operands will land at the lowest bits of the result (right-most if you write the number down). This tries to reflect how Verilog concatenation works ({a, b}), but is a bit unfortunate and has caused a lot of pain and bugs in various parts of CIRCT 🙈.

Also would using a different dialect such as mlir::llvm not be viable here with its built in mlir::llvm::zext function?

From an MLIR perspective, and when you want to do something like simulate the design, that would definitely work. The MooreToCore conversion tries to only use operations in the HW, Comb, Seq, LLHD, Sim, Verif, and LTL dialects, though. So producing an LLVM dialect operation would make it harder for other parts of CIRCT to then process the IR further.

Just maybe another small question as well; are both input integers guaranteed to be of the same type width, or would you recommend I look for length of the bigger one and convert the other side to the same width?

Good point, and yes, they are! The PowUOp is based on BinaryOpBase, which has the SameOperandsAndResultType trait:

circt/include/circt/Dialect/Moore/MooreOps.td

Lines 722 to 723 in 56625bf

	class BinaryOpBase<string mnemonic, list<Trait> traits = []> :
	MooreOp<mnemonic, traits # [Pure, SameOperandsAndResultType]> {

[liamslj13]

The comb.concat operands go from most significant bit to least significant bit. So the first operands will land at the highest bits of the result (left-most if you write the number down), and the last operands will land at the lowest bits of the result (right-most if you write the number down). This tries to reflect how Verilog concatenation works ({a, b}), but is a bit unfortunate and has caused a lot of pain and bugs in various parts of CIRCT 🙈.

Alright, thank you for clearing this up! I don't exactly understand SystemVerilog/Verilog or other verification methods very well yet so please forgive me lol, the last part may have gone over my head.

I will keep a note on the dialect usage as well so I know what is permissible to use unless specified otherwise for next time.

I have added the suggestions you have given me; thank you as always! I also rewrote the test in basic.mlir. The FileCheck variable(?) names or other conventions may be off, but I tried to mimic what other tests (bigger than PowSOp) were doing. Everything should run okay.

[fabianschuiki]

Very cool, this looks great! Thanks a lot for all your work 🚀. If it's okay with you, I'll merge this if CI is happy.

[liamslj13]

Very cool, this looks great! Thanks a lot for all your work 🚀. If it's okay with you, I'll merge this if CI is happy.

All good with me 👍 Thank you for your patience with me and the explanations on the codebase. Glad it was all able to get done smoothly!