dialects: (llvm) added the 'exact' keyword to the corresponding operations #3390
Conversation
tobiasgrosser
changed the title
|
@lfrenot, can you rebase to ensure this PR has a clean history? |
`mlir-opt` attempts to print floats as 6dp scientific notation, but round-trips to ensure there is no precision loss and the printed number will bitwise reproduce exactly. If this fails, it will choose a different printing method to ensure bit-reproducible printing for a given precision of fp types. This PR takes the approach of mlir-opt: attempt to print to scientific 6dp notation iff the resulting string is losslessly reproducible, or else print to full precision using repr. This has the advantage of not affecting a vast number of filechecks, mirroring upstream, and not having a flag for either full or reduced precision. --------- Co-authored-by: n-io <n-io@users.noreply.github.com>
This prints `FloatAttr` special values `nan`, `inf`, `-inf` as a hexadecimal binary representation to align with mlir-opt as described in Also see the [FloatAttr docs](https://mlir.llvm.org/docs/Dialects/Builtin/#floatattr) or give it a try on [godbolt](https://godbolt.org/z/89TfsvaKT). --------- Co-authored-by: n-io <n-io@users.noreply.github.com>
lfrenot
force-pushed
the
leon/llvm-exact-flag
branch
from
fba3661 to
442650a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3390 +/- ##
==========================================
+ Coverage 90.13% 90.15% +0.02%
==========================================
Files 452 452
Lines 57157 57195 +38
Branches 5498 5500 +2
==========================================
+ Hits 51519 51565 +46
+ Misses 4180 4175 -5
+ Partials 1458 1455 -3 ☔ View full report in Codecov by Sentry. |
|
You still seem to have unrelated changes in the commits, no? |
There was a problem hiding this comment.
LGTM modulo prop_name. @tobiasgrosser, which are the extra files you mean?
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
xdsl/dialects/llvm.py
Outdated
| lhs = operand_def(T) | ||
| rhs = operand_def(T) | ||
| res = result_def(T) | ||
| is_exact = opt_prop_def(BoolAttr, prop_name="isExact") |
There was a problem hiding this comment.
Oh, one more thing, is that I don't think it makes sense for this property to be optional, isn't it the same as default 0?
| is_exact = opt_prop_def(BoolAttr, prop_name="isExact") | |
| is_exact = prop_def(BoolAttr, prop_name="isExact") |
There was a problem hiding this comment.
also, default_value might be useful here
No extra files, but 6460b2d is included in the list of commits here. We should avoid this in the future and start with clean PR histories. Given that this PR is almost closed, we are fine, I guess. |
|
When we merge in xDSL, the history is deleted, as the merges are rebase+squash, so rebasing is not a pattern I've seen very much. |
|
These wouldn't work right now, since exact flags are not in MLIR yet |
Sure, but PR start typically with a clean history and not with some random other PRs being part of it. |
|
@tobiasgrosser are we good to merge? |
|
Yes, good to merge if @superlopuh is happy. |
…tions (xdslproject#3390) Co-authored-by: Nicolai Stawinoga <36768051+n-io@users.noreply.github.com> Co-authored-by: n-io <n-io@users.noreply.github.com> Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
No description provided.