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

ZIR-212: restore eval's skip-terminator behavior; bibc decoder adds ReturnOp #51

Closed
wants to merge 1 commit into from

Conversation

mars-risc0
Copy link
Contributor

@mars-risc0 mars-risc0 commented Oct 28, 2024

BigInt::eval formerly tried to skip a BigInt function's terminator op, using without_terminator(), but this appeared to be an error, because the BigInt dialect does not define any terminator ops. This came up because the last operation (usually an EqZ) would be skipped when running the evaluator on functions decoded from bytecode. I therefore deleted the without_terminator() call, in one of the later changes rolled into aac967b.

Looking deeper, however, this leaves us with an inconsistency: the edsl finishes each BigInt function off with a ReturnOp, which the bibc encoder skips; the bibc decoder does not re-insert this ReturnOp when reconstituting bytecode into MLIR ops, and that is why the evaluator was skipping the last real operation instead.

This change makes it all consistent again: the edsl continues to add a ReturnOp which everyone else will ignore, the bibc decoder now also inserts such an op for consistency, and the evaluator once again skips the ReturnOp instead of throwing.

(Deleting the ReturnOp everywhere would also be a reasonable solution to this problem, but the edsl is older than the bibc system, so I am deferring to its design here.)

@mars-risc0 mars-risc0 requested review from shkoo and jbruestle October 28, 2024 15:27
@github-actions github-actions bot changed the title restore eval's skip-terminator behavior; bibc decoder adds ReturnOp ZIR-211: restore eval's skip-terminator behavior; bibc decoder adds ReturnOp Oct 28, 2024
@github-actions github-actions bot changed the title ZIR-211: restore eval's skip-terminator behavior; bibc decoder adds ReturnOp ZIR-212: restore eval's skip-terminator behavior; bibc decoder adds ReturnOp Oct 28, 2024
@mars-risc0 mars-risc0 requested a review from flaub October 28, 2024 15:28
@mars-risc0 mars-risc0 force-pushed the mars/consistent-bigint-func-terminators branch from 3580a99 to d5c8dee Compare October 28, 2024 19:15
@mars-risc0 mars-risc0 enabled auto-merge (squash) October 28, 2024 19:30
@mars-risc0 mars-risc0 force-pushed the mars/consistent-bigint-func-terminators branch from d5c8dee to c098734 Compare October 28, 2024 21:05
@mars-risc0
Copy link
Contributor Author

already merged as a prerequisite for ZIR-215

@mars-risc0 mars-risc0 closed this Oct 29, 2024
auto-merge was automatically disabled October 29, 2024 21:24

Pull request was closed

@mars-risc0 mars-risc0 deleted the mars/consistent-bigint-func-terminators branch December 9, 2024 23:14
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.

1 participant