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

Codegen emits non-bool cir.cmp operations #1108

Closed
orbiri opened this issue Nov 11, 2024 · 4 comments · Fixed by #1110
Closed

Codegen emits non-bool cir.cmp operations #1108

orbiri opened this issue Nov 11, 2024 · 4 comments · Fixed by #1110
Assignees
Labels
IR difference A difference in ClangIR-generated LLVM IR that could complicate reusing original CodeGen tests

Comments

@orbiri
Copy link
Collaborator

orbiri commented Nov 11, 2024

When looking at some C generated code I noticed that most cir.cmp operations that were generated were of type s32i, and then immediately converted to bool before used in if statements etc.

I found the root cause in CIRGenScalarExpr, will publish a fix over the coming week that simplifies the generated code for most cases.

———

I will note that a lot of tests rely on 1:1 generated CIR/LLVMIR, even without using file check variables, which makes such changes a lot harder than they could have been! For example, tests of lowering of complicated scopes and control flow should not check for the way that conditions are evaluated.

@smeenai
Copy link
Collaborator

smeenai commented Nov 11, 2024

This might be related to #480?

@bcardosolopes
Copy link
Member

found the root cause in CIRGenScalarExpr, will publish a fix over the coming week that simplifies the generated code for most cases.

Nice!

tests of lowering of complicated scopes and control flow should not check for the way that conditions are evaluated

+1

This might be related to #480?

Are there cases related with i8-i1 as well?

@orbiri
Copy link
Collaborator Author

orbiri commented Nov 12, 2024

Uploaded fix + WIP tests in #1110

Still need to add some nice documentation, real commitdesc etc, and make all tests pass :)

@orbiri
Copy link
Collaborator Author

orbiri commented Nov 12, 2024

Unfortunately since #480 is in C++ this issue is unrelated.

@seven-mile seven-mile added the IR difference A difference in ClangIR-generated LLVM IR that could complicate reusing original CodeGen tests label Nov 14, 2024
@orbiri orbiri linked a pull request Nov 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IR difference A difference in ClangIR-generated LLVM IR that could complicate reusing original CodeGen tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants