Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MooreToCore] Support pows and powu op #7899

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Max-astro
Copy link
Contributor

Fixes #7626

@Max-astro
Copy link
Contributor Author

Hi @fabianschuiki , I have incorporated the advice from you and @maerhart . Please let me know if there are any issues with my code or the test cases.

Thank you so much for taking the time to review this. I really appreciate your feedback! :)

Comment on lines +1186 to +1193
auto intType = cast<IntType>(op.getRhs().getType());
if (auto baseOp = op.getLhs().getDefiningOp<ConstantOp>()) {
if (baseOp.getValue() == 2) {
Value constOne = rewriter.create<ConstantOp>(loc, intType, 1);
rewriter.replaceOpWithNewOp<ShlOp>(op, constOne, op.getRhs());
return success();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be fantastic to have as a canonicalization in the Moore dialect! Could you move this into the canonicalizer of moore::PowUOp? (You might have to add a let hasCanonicalizeMethod = 1 to the PowUOp definition in the MooreOps.td file.)

Comment on lines +1228 to +1245
auto intType = cast<IntType>(op.getRhs().getType());
if (auto baseOp = op.getLhs().getDefiningOp<ConstantOp>()) {
if (baseOp.getValue() == 2) {
Value constOne = rewriter.create<ConstantOp>(loc, intType, 1);
Value constZero = rewriter.create<ConstantOp>(loc, intType, 0);
Value shift = rewriter.create<ShlOp>(loc, constOne, op.getRhs());
Value isNegative = rewriter.create<SltOp>(loc, op.getRhs(), constZero);
auto condOp = rewriter.replaceOpWithNewOp<ConditionalOp>(
op, op.getLhs().getType(), isNegative);
Block *thenBlock = rewriter.createBlock(&condOp.getTrueRegion());
rewriter.setInsertionPointToStart(thenBlock);
rewriter.create<YieldOp>(loc, constZero);
Block *elseBlock = rewriter.createBlock(&condOp.getFalseRegion());
rewriter.setInsertionPointToStart(elseBlock);
rewriter.create<YieldOp>(loc, shift);
return success();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be fantastic to have as a canonicalization in the Moore dialect! Could you move this into the canonicalizer of moore::PowSOp? (You might have to add a let hasCanonicalizeMethod = 1 to the PowSOp definition in the MooreOps.td file.)

Comment on lines +1255 to +1262
// if the exponent is negative, lhsVal = 1 / lhs, upperBound = -rhs
Value invLhs = rewriter.create<comb::DivUOp>(
loc, rewriter.create<hw::ConstantOp>(loc, resultType, 1), lhsVal);
Value negRhs = rewriter.create<comb::SubOp>(loc, constZero, rhsVal);

lhsVal = rewriter.create<comb::MuxOp>(loc, isNegative, invLhs, lhsVal);
Value upperBound =
rewriter.create<comb::MuxOp>(loc, isNegative, negRhs, rhsVal);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the exponent is negative, can this ever produce anything besides 0? Since these are integers, I'd assume 1 / lhs to always return 0, since there are no fractional numbers. We'll have to figure out a way to represent real numbers, but you could check if the PowSOp type is a moore::IntType, and only implement integers for now. (We don't really have a lowering for SystemVerilog's real type yet. That would be cool to add at some point -- but we'd probably have to map that to the arith MLIR dialect and MLIR's standard floating-point type.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MooreToCore] Support pow op
2 participants