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] Extend support for floating point attributes #572

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

orbiri
Copy link
Contributor

@orbiri orbiri commented Apr 28, 2024

This commit extends the support for floating point attributes parsing by using
the new AsmParser::parseFloat(fltSemnatics, APFloat&) interface.
As a drive-by, this commit also harmonizes the cir.fp print/parse namespace
usage, and adds the constraint of supporting only "CIRFPType"s for cir.fp in
tablegen instead of verifying it manually in the parsing logic.


This commit is based on top of a to-be-upstreamed commit which extends the upstream MLIR float type parsing. Upstream parsing of float type has full capability only through parsing the Builtin Dialect's FloatAttr. Thos commit exposes the same capabilities to downstream users.


This PR should resolve (at least) GCC-C-execute-ieee-fp-cmp-2 and GCC-C-execute-ieee-fp-cmp-4, paving the way to other GCC-C-execute-ieee-* tests passing from the SingleSource suite. It resolves #559 .

@orbiri
Copy link
Contributor Author

orbiri commented Apr 28, 2024

@Lancern - may be related to work that you are working on 🙏 (e.g. #536 #571 ).

@orbiri
Copy link
Contributor Author

orbiri commented Apr 29, 2024

Upstream patch: llvm/llvm-project#90442

@lanza lanza force-pushed the main branch 2 times, most recently from c4db6d0 to e197d4e Compare April 29, 2024 20:11
@bcardosolopes
Copy link
Member

Nathan just did a rebase, can you please update? Sorry for the churn

orbiri added 2 commits May 1, 2024 09:53
Parsing support for floating point types was missing a few features:
1. Parsing floating point attributes from integer literals was supported only
   for types with bitwidth smaller or equal to 64.
2. Downstream users could not use `AsmParser::parseFloat` to parse float types
   which are printed as integer literals.

This commit addresses both these points. It extends
`Parser::parseFloatFromIntegerLiteral` to support arbitrary bitwidth, and
exposes a new API to parse arbitrary floating point given an fltSemantics as
input. The usage of this new API is introduced in the Test Dialect.
This commit extends the support for floating point attributes parsing by using
the new `AsmParser::parseFloat(fltSemnatics, APFloat&)` interface.
As a drive-by, this commit also harmonizes the cir.fp print/parse namespace
usage, and adds the constraint of supporting only "CIRFPType"s for cir.fp in
tablegen instead of verifying it manually in the parsing logic.
@orbiri
Copy link
Contributor Author

orbiri commented May 1, 2024

Rebased 🙏
Update on upstream commit: PR was approved, tying to wait a bit more for the core developers who most recently touched that code (Jeff & River) to take a look. Will merge by Friday if no further comments arise.

@bcardosolopes
Copy link
Member

Thanks for going the extra length to make sure this works!

@orbiri
Copy link
Contributor Author

orbiri commented May 5, 2024

Upstream patch: llvm/llvm-project#90442

Merged upstream 🎉
Waiting for the next CIR rebase and I'll update this one.

@orbiri
Copy link
Contributor Author

orbiri commented May 17, 2024

@bcardosolopes - is there any planned rebase anytime soon? This PR is blocked by getting the upstream patch. Alternatively, I assume we can cherry-pick it here and there's a 95% chance that it will dissolve on the next rebase. Let me know what you prefer 🙏🏼

@bcardosolopes
Copy link
Member

@bcardosolopes - is there any planned rebase anytime soon? This PR is blocked by getting the upstream patch. Alternatively, I assume we can cherry-pick it here and there's a 95% chance that it will dissolve on the next rebase. Let me know what you prefer 🙏🏼

@lanza wdyt?

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.

Global "NaN" Is Lowered to 0.0 During LLVM Conversion
2 participants