core: Parse FloatAttr from hexadecimal bit representation #3378
Conversation
xdsl/parser/attribute_parser.py
Outdated
| hexadecimal_token: Token | None = None | ||
| if self._current_token.text[:2] in ["0x", "0X"]: | ||
| hexadecimal_token = self._current_token | ||
|
|
There was a problem hiding this comment.
I'm not sure this is the best way to do this, but it prevents losing the information that the token is a hexadecimal literal instead of a generic integer, until the attribute type is parsed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3378 +/- ##
==========================================
- Coverage 90.13% 90.13% -0.01%
==========================================
Files 452 452
Lines 57134 57158 +24
Branches 5493 5498 +5
==========================================
+ Hits 51499 51520 +21
Misses 4180 4180
- Partials 1455 1458 +3 ☔ View full report in Codecov by Sentry. |
xdsl/utils/lexer.py
Outdated
| if self.kind == Token.Kind.INTEGER_LIT and self.text[:2] in ["0x", "0X"]: | ||
| match len(self.text): | ||
| # the number corresponds to `len('0x') + (float_bitwidth / 4)` | ||
| case 6: | ||
| return struct.unpack("<e", struct.pack("<H", int(self.text, 16)))[0] | ||
| case 10: | ||
| return struct.unpack("<f", struct.pack("<I", int(self.text, 16)))[0] | ||
| case 18: | ||
| return struct.unpack("<d", struct.pack("<Q", int(self.text, 16)))[0] | ||
| case _: | ||
| # todo support further bitwidths | ||
| pass |
There was a problem hiding this comment.
shouldn't the bitwidth be determined by the floating point type? I would expect this to be in a helper function that takes an int and a bitwidth, possibly in the bitwise_casts file, with dedicated tests, and for the logic to be done inline in parse_optional_number
There was a problem hiding this comment.
Good spot, let me fix that
There was a problem hiding this comment.
Hadn't really considered implied leading zeros, but mlir-opt supports it
`FloatAttr`s can have their values specified by a hexadecimal literal. This should be interpreted as the bit representation of the float. However, it is currently interpreted as an int number and then converted to a float, leading to incorrect behaviour. This PR is the second part of #3370 after #3377 --------- Co-authored-by: n-io <n-io@users.noreply.github.com>
…t#3378) `FloatAttr`s can have their values specified by a hexadecimal literal. This should be interpreted as the bit representation of the float. However, it is currently interpreted as an int number and then converted to a float, leading to incorrect behaviour. This PR is the second part of xdslproject#3370 after xdslproject#3377 --------- Co-authored-by: n-io <n-io@users.noreply.github.com>
FloatAttrs can have their values specified by a hexadecimal literal. This should be interpreted as the bit representation of the float. However, it is currently interpreted as an int number and then converted to a float, leading to incorrect behaviour.This PR is the second part of #3370 after #3377