Skip to content

[ThroughMLIR] basic printf support #1687

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

Merged
merged 4 commits into from
Jun 17, 2025
Merged

Conversation

felixdaas
Copy link
Contributor

This PR is related to #1685 and adds some basic support for the printf function.

Limitations:

  1. It only works if all variadic params are of basic interger/float type (for more info why memref type operands don't work see [ThroughMLIR] variadic functions (and printf) support #1685)
  2. Only works if the format string is definied directly inside the printf function

The downside of this PR is also that the handling this edge case adds significant code bloat and reduces readability for the cir.call op lowering (I tried to insert some meanigful comments to improve the readability), but I think its worth to do this so we have some basic printf support (without adding an extra cir operation) until upstream support for variadic functions is added to the func dialect. Also a few more test (which use such a basic form of printf) in the llvm Single Source test suite are working with this PR:

before this PR:

Testing Time: 4.00s

Total Discovered Tests: 1833
Passed : 420 (22.91%)
Failed : 10 (0.55%)
Executable Missing: 1403 (76.54%)

with this PR:

Testing Time: 10.29s

Total Discovered Tests: 1833
Passed : 458 (24.99%)
Failed : 6 (0.33%)
Executable Missing: 1369 (74.69%)

@felixdaas felixdaas marked this pull request as ready for review June 15, 2025 11:59
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Thanks!

@bcardosolopes bcardosolopes merged commit 1ca3376 into llvm:main Jun 17, 2025
12 checks passed
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