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

[CIR] Force cir.cmp to always return bool #1110

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

Conversation

orbiri
Copy link
Collaborator

@orbiri orbiri commented Nov 12, 2024

It was always the intention for cir.cmp operations to return bool result. Due
to missing constraints, a bug in codegen has slipped in which created cir.cmp
operations with result type that matches the original AST expression type. In
C, as opposed to C++, boolean expression types are "int". This resulted with
extra operations being codegened around boolean expressions and their usage.

This commit both enforces cir.cmp in the op definition and fixes the
mentioned bug.

@bcardosolopes
Copy link
Member

Approach sounds good, thank you!

It was always the intention for `cir.cmp` operations to return bool result. Due
to missing constraints, a bug in codegen has slipped in which created `cir.cmp`
operations with result type that matches the original AST expression type. In
C, as opposed to C++, boolean expression types are "int". This resulted with
extra operations being codegened around boolean expressions and their usage.

This commit both enforces `cir.cmp` in the op definition and fixes the
mentioned bug.
@orbiri orbiri marked this pull request as ready for review November 15, 2024 21:40
@orbiri orbiri changed the title [WIP] Always generate cir.cmp with boolean result [CIR] Force cir.cmp to always return bool Nov 15, 2024
@orbiri
Copy link
Collaborator Author

orbiri commented Nov 15, 2024

Ready for review :)

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