ISLE: support more flexible integer constants. #4559
Conversation
alexcrichton
left a comment
There was a problem hiding this comment.
Seems reasonable to me, but I have a mild amount of hesitation with this. IIRC this is how the Rust AST used to look like (more or less) and eventually it changed to a much different representation where I think everything is stored as u128 and the negation bit is stored separately. The reason and rationale for that change though may be around providing warnings about "this literal is too big for this type" than anything else perhaps. I just know that handling literal integers in an AST is surprisingly tricky...
In any case this is better than before and seems like it should work just fine!
Subscribe to Label ActionDetailsThis issue or pull request has been labeled: "cranelift", "isle"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
The ISLE language's lexer previously used a very primitive `i64::from_str_radix` call to parse integer constants, allowing values in the range -2^63..2^63 only. Also, underscores to separate digits (as is allwoed in Rust) were not supported. Finally, 128-bit constants were not supported at all. This PR addresses all issues above: - Integer constants are internally stored as 128-bit values. - Parsing supports either signed (-2^127..2^127) or unsigned (0..2^128) range. Negation works independently of that, so one can write `-0xffff..ffff` (128 bits wide, i.e., -(2^128-1)) to get a `1`. - Underscores are supported to separate groups of digits, so one can write `0xffff_ffff`. - A minor oversight was fixed: hex constants can start with `0X` (uppercase) as well as `0x`, for consistency with Rust and C. This PR also adds a new kind of ISLE test that actually runs a driver linked to compiled ISLE code; we previously didn't have any such tests, but it is now quite useful to assert correct interpretation of constant values.
da6be17 to
bf5ddef
Compare
|
Indeed, there's more we could/should do if we want to properly warn about overflows. ISLE's type system isn't quite sophisticated enough to do that at the moment though, as it doesn't actually know about the integral types at a built-in level (mostly); so for now the model is "we hold 128 bits and we pass them through". And the codegen does an explicit |
|
(Pushed a few formatting changes, no substantial diff.) |
The ISLE language's lexer previously used a very primitive `i64::from_str_radix` call to parse integer constants, allowing values in the range -2^63..2^63 only. Also, underscores to separate digits (as is allwoed in Rust) were not supported. Finally, 128-bit constants were not supported at all. This PR addresses all issues above: - Integer constants are internally stored as 128-bit values. - Parsing supports either signed (-2^127..2^127) or unsigned (0..2^128) range. Negation works independently of that, so one can write `-0xffff..ffff` (128 bits wide, i.e., -(2^128-1)) to get a `1`. - Underscores are supported to separate groups of digits, so one can write `0xffff_ffff`. - A minor oversight was fixed: hex constants can start with `0X` (uppercase) as well as `0x`, for consistency with Rust and C. This PR also adds a new kind of ISLE test that actually runs a driver linked to compiled ISLE code; we previously didn't have any such tests, but it is now quite useful to assert correct interpretation of constant values.
The ISLE language's lexer previously used a very primitive
i64::from_str_radixcall to parse integer constants, allowing valuesin the range -2^63..2^63 only. Also, underscores to separate digits (as
is allwoed in Rust) were not supported. Finally, 128-bit constants were
not supported at all.
This PR addresses all issues above:
range. Negation works independently of that, so one can write
-0xffff..ffff(128 bits wide, i.e., -(2^128-1)) to get a1.write
0xffff_ffff.0X(uppercase) as well as
0x, for consistency with Rust and C.This PR also adds a new kind of ISLE test that actually runs a driver
linked to compiled ISLE code; we previously didn't have any such tests,
but it is now quite useful to assert correct interpretation of constant
values.