ISLE: Add proper bool type#9593
Conversation
2db83b3 to
7d0993f
Compare
Subscribe to Label ActionDetailsThis issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "isle"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
cfallin
left a comment
There was a problem hiding this comment.
Thanks for working on this -- it's nice to see motion on the "nice-to-have" upgrades we've had in our backlog for a while!
Mostly looks fine; my main thought is about the BuiltinType abstraction and whether we can separate the change out.
|
|
||
| /// A built-in type. | ||
| #[derive(Copy, Clone, Debug, PartialEq, Eq)] | ||
| pub enum BuiltinType { |
There was a problem hiding this comment.
I'm not sure I understand the reason for having a notion of "built-in types" for bool but not for other types that we have constants for (namely integers) -- while on the other hand it adds some complication to the IR. Maybe we could separate out this part into another PR and discuss separately?
There was a problem hiding this comment.
(To make it more explicit: I think we probably should have this eventually; but for integers too, and then we should check that integer literals are used only in contexts that expect them.)
There was a problem hiding this comment.
The intention is to extend BuiltinType to include other types (like u{8,16,32,64,size}, i{8,16,32,64,size} or str) in future PRs
There was a problem hiding this comment.
OK, yeah, on further thought I suppose it's totally fine to migrate integers over as a second step -- as long as the intention is there to finish out the refactor. Thanks!
Copyright (c) 2024, Arm Limited. Signed-off-by: Karl Meakin <karl.meakin@arm.com>
Copyright (c) 2024, Arm Limited. Signed-off-by: Karl Meakin <karl.meakin@arm.com>
Copyright (c) 2024, Arm Limited. Signed-off-by: Karl Meakin <karl.meakin@arm.com>
Copyright (c) 2024, Arm Limited. Replace all occurences of `$true` and `$false` with `true` and `false`. Signed-off-by: Karl Meakin <karl.meakin@arm.com>
Instead of threading `on_lhs` through all the calls to `translate_expr`, we can just set `is_partial` and `is_pure` on `root_flags` to true. Copyright (c) 2024, Arm Limited. Signed-off-by: Karl Meakin <karl.meakin@arm.com>
Copyright (c) 2024, Arm Limited. Signed-off-by: Karl Meakin <karl.meakin@arm.com>
7d0993f to
e2f2d70
Compare
Partially solves #3573