Skip to content

Add ModArithType folding hooks#3067

Merged
copybara-service[bot] merged 1 commit into
mainfrom
test_931382239
Jun 17, 2026
Merged

Add ModArithType folding hooks#3067
copybara-service[bot] merged 1 commit into
mainfrom
test_931382239

Conversation

@copybara-service

Copy link
Copy Markdown
Contributor

Add ModArithType folding hooks

This change extends mod_arith folders to support RNS using the new Limbwise- attribute and op interfaces

Part of #97

@j2kun j2kun requested a review from crockeea June 13, 2026 01:31
@j2kun

j2kun commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Note this is rebased over #3065 but the commits get squashed by copybara.

res = lw + rw;
break;
case ModOp::Sub:
res = lw + mw - rw.urem(mw);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why add the modulus here? Is there an assumption somewhere that values should not be negative? c.f. my confusion here

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see the urem below... any other reason?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ignoring the urem at the end, I think until ModArithToArith is updated to support anything except standard representatives in [0, q), we will have to maintain that invariant in the folders or else the lowerings may break.

auto rnsType = dyn_cast_if_present<RNSType>(type);
if (!rnsType) return nullptr;
auto op = rns::ConstantOp::create(builder, loc, rnsType, *this);
return op.getOperation();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How is that different than return op?

@j2kun j2kun Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What you wrote indeed compiles, but that's because, technically speaking, the tablegen-generated classes like ConstantOp inherit from ::mlir::Op (cf here) which wrap an Operation *, and these classes have an implicit typecaster from ::mlir::Op to Operation *. Usually when I explicitly want an Operation *, however, I'll try to make it more explicit and ask for the Operation * explicitly.

@crockeea

Copy link
Copy Markdown
Collaborator

Mostly questions for my own understanding. Love where this is headed!

@copybara-service copybara-service Bot force-pushed the test_931382239 branch 4 times, most recently from b95aa11 to b88f293 Compare June 17, 2026 04:24
This change extends mod_arith folders to support RNS using the new Limbwise- attribute and op interfaces

Part of #97

PiperOrigin-RevId: 933486917
@copybara-service copybara-service Bot merged commit 1119f1f into main Jun 17, 2026
@copybara-service copybara-service Bot deleted the test_931382239 branch June 17, 2026 05:01
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.

2 participants