Skip to content

cranelift: ISLE wrapper for constructing constants for integers, floating-points and vectors #11942

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+171 −64
Open

cranelift: ISLE wrapper for constructing constants for integers, floating-points and vectors #11942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57c2f25
cranelift: ISLE wrapper for constructing constants
Gonzalosilvalde Apr 5, 2025
522bdee
cranelift: replace iconst to const for RHS rules
efJerryYang Oct 8, 2025
418191b
cranelift: add ieee conversion helpers and rules for float/vector con…
efJerryYang Oct 25, 2025
7da9069
cranelift: clean up redundant code and update comments
efJerryYang Oct 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions cranelift/codegen/src/isle_prelude.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,36 @@ macro_rules! isle_common_prelude_methods {
val
}

#[inline]
fn ieee16_from_imm64(&mut self, val: Imm64) -> Ieee16 {
Ieee16::with_bits(val.bits() as u16)
}

#[inline]
fn ieee32_from_imm64(&mut self, val: Imm64) -> Ieee32 {
Ieee32::with_bits(val.bits() as u32)
}

#[inline]
fn ieee64_from_imm64(&mut self, val: Imm64) -> Ieee64 {
Ieee64::with_bits(val.bits() as u64)
}

#[inline]
fn imm64_from_ieee16(&mut self, val: Ieee16) -> Imm64 {
Imm64::new(val.bits() as i64)
}

#[inline]
fn imm64_from_ieee32(&mut self, val: Ieee32) -> Imm64 {
Imm64::new(val.bits() as i64)
}

#[inline]
fn imm64_from_ieee64(&mut self, val: Ieee64) -> Imm64 {
Imm64::new(val.bits() as i64)
}
Comment on lines +551 to +579
Copy link
Member

@fitzgen fitzgen Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid as casts here? Instead, we should use .cast_[un]signed() and {i,u}64::try_from(...).unwrap() as necessary. This makes the behavior we intend explicit.

If we expect that the high bits might be set, and we want to ignore them anyways, then we should explicitly mask them away via Imm64::{zero,sign}_extend_from_width before calling i64::try_from(...).unwrap() and then feeding the bits into Ieee64::with_bits et al.

Otherwise, if the high bits should never be set, then we should just use i64::try_from(...).unwrap() directly.

Copy link
Contributor Author

@efJerryYang efJerryYang Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this comment! I will keep in mind to use the correct cast later when contributing to this repo, but according to what you have mentioned at the very beginning comment, we will not include the changes for float/vec, is that correct?

Copy link
Member

@fitzgen fitzgen Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will not include the changes for float/vec, is that correct?

Correct. The review has my thoughts at a couple of different points in time, apologies for that.


fn not_vec32x2(&mut self, ty: Type) -> Option<Type> {
if ty.lane_bits() == 32 && ty.lane_count() == 2 {
None
Expand Down
130 changes: 66 additions & 64 deletions cranelift/codegen/src/opts/cprop.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,96 +3,96 @@
(rule (simplify
(clz (fits_in_64 ty)
(iconst ty kx)))
(subsume (iconst ty (imm64_clz ty kx))))
(subsume (const ty (imm64_clz ty kx))))


(rule (simplify
(ctz (fits_in_64 ty)
(iconst ty kx)))
(subsume (iconst ty (imm64_ctz ty kx))))
(subsume (const ty (imm64_ctz ty kx))))

(rule (simplify
(iadd (fits_in_64 ty)
(iconst ty (u64_from_imm64 k1))
(iconst ty (u64_from_imm64 k2))))
(subsume (iconst ty (imm64_masked ty (u64_wrapping_add k1 k2)))))
(subsume (const ty (imm64_masked ty (u64_wrapping_add k1 k2)))))

(rule (simplify
(isub (fits_in_64 ty)
(iconst ty (u64_from_imm64 k1))
(iconst ty (u64_from_imm64 k2))))
(subsume (iconst ty (imm64_masked ty (u64_wrapping_sub k1 k2)))))
(subsume (const ty (imm64_masked ty (u64_wrapping_sub k1 k2)))))
Comment on lines 20 to +24
Copy link
Member

@fitzgen fitzgen Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a simple example of how the updated usage doesn't really seem to be an improvement. We are still passing the unwieldy Imm64 type around and explicitly masking to the relevant type, the only difference is we are doing const instead of iconst.

For this kind of change to be an actual improvement, rather than just moving deck chairs around and being size-of-one and half-a-dozen-of-another, I think we would want to have this rule's RHS be something like this:

(subsume (my_const ty (u64_wrapping_sub k1 k2)))

Where my_const is whatever the new constant constructor for integer types is called.

Specifically, this folds the imm64_masked call into the helper, which gives us two benefits over the current status quo:

  1. We are dealing with u64 instead of Imm64, which is what all of the cprop intermediate computations actually use.
  2. We don't need to think about type masking as much ourselves nor repeatedly pass ty around in multiple places.


(rule (simplify
(imul (fits_in_64 ty)
(iconst ty (u64_from_imm64 k1))
(iconst ty (u64_from_imm64 k2))))
(subsume (iconst ty (imm64_masked ty (u64_wrapping_mul k1 k2)))))
(subsume (const ty (imm64_masked ty (u64_wrapping_mul k1 k2)))))

(rule (simplify_skeleton
(sdiv (iconst ty k1)
(iconst ty k2)))
(if-let d (imm64_sdiv ty k1 k2))
(iconst ty d))
(const ty d))

(rule (simplify_skeleton
(srem (iconst ty k1)
(iconst ty k2)))
(if-let d (imm64_srem ty k1 k2))
(iconst ty d))
(const ty d))

(rule (simplify_skeleton
(udiv (iconst_u ty k1)
(iconst_u ty k2)))
(if-let d (u64_checked_div k1 k2))
(iconst ty (imm64_masked ty d)))
(const ty (imm64_masked ty d)))

(rule (simplify_skeleton
(urem (iconst_u ty k1)
(iconst_u ty k2)))
(if-let d (u64_checked_rem k1 k2))
(iconst ty (imm64_masked ty d)))
(const ty (imm64_masked ty d)))

(rule (simplify
(bor (fits_in_64 ty)
(iconst ty (u64_from_imm64 k1))
(iconst ty (u64_from_imm64 k2))))
(subsume (iconst ty (imm64_masked ty (u64_or k1 k2)))))
(subsume (const ty (imm64_masked ty (u64_or k1 k2)))))

(rule (simplify
(band (fits_in_64 ty)
(iconst ty (u64_from_imm64 k1))
(iconst ty (u64_from_imm64 k2))))
(subsume (iconst ty (imm64_masked ty (u64_and k1 k2)))))
(subsume (const ty (imm64_masked ty (u64_and k1 k2)))))

(rule (simplify
(bxor (fits_in_64 ty)
(iconst ty (u64_from_imm64 k1))
(iconst ty (u64_from_imm64 k2))))
(subsume (iconst ty (imm64_masked ty (u64_xor k1 k2)))))
(subsume (const ty (imm64_masked ty (u64_xor k1 k2)))))

(rule (simplify
(bnot (fits_in_64 ty)
(iconst ty (u64_from_imm64 k))))
(subsume (iconst ty (imm64_masked ty (u64_not k)))))
(subsume (const ty (imm64_masked ty (u64_not k)))))

(rule (simplify (ishl (fits_in_64 ty)
(iconst ty k1)
(iconst _ k2)))
(subsume (iconst ty (imm64_shl ty k1 k2))))
(subsume (const ty (imm64_shl ty k1 k2))))

(rule (simplify (ushr (fits_in_64 ty)
(iconst ty k1)
(iconst _ k2)))
(subsume (iconst ty (imm64_ushr ty k1 k2))))
(subsume (const ty (imm64_ushr ty k1 k2))))

(rule (simplify (sshr (fits_in_64 ty)
(iconst ty k1)
(iconst _ k2)))
(subsume (iconst ty (imm64_sshr ty k1 k2))))
(subsume (const ty (imm64_sshr ty k1 k2))))

(rule (simplify (ireduce narrow (iconst (fits_in_64 _) (u64_from_imm64 imm))))
(subsume (iconst narrow (imm64_masked narrow imm))))
(subsume (const narrow (imm64_masked narrow imm))))

;; iconst_[su] support $I128, but do so by extending, so restricting to
;; 64-bit or smaller keeps it from just remaking essentially the same thing.
Expand All @@ -106,7 +106,7 @@
cc
(iconst ty k1)
(iconst ty k2)))
(subsume (iconst result_ty (imm64_icmp ty cc k1 k2))))
(subsume (const result_ty (imm64_icmp ty cc k1 k2))))


;; Canonicalize via commutativity: push immediates to the right.
Expand Down Expand Up @@ -225,17 +225,19 @@
;; A splat of a constant can become a direct `vconst` with the appropriate bit
;; pattern.
(rule (simplify (splat (ty_vec128 dst) (iconst $I8 n)))
(vconst dst (splat8 (u64_uextend_imm64 $I8 n))))
(const dst n))
(rule (simplify (splat (ty_vec128 dst) (iconst $I16 n)))
(vconst dst (splat16 (u64_uextend_imm64 $I16 n))))
(const dst n))
(rule (simplify (splat (ty_vec128 dst) (iconst $I32 n)))
(vconst dst (splat32 (u64_uextend_imm64 $I32 n))))
(const dst n))
(rule (simplify (splat (ty_vec128 dst) (iconst $I64 n)))
(vconst dst (splat64 (u64_uextend_imm64 $I64 n))))
(const dst n))
(rule (simplify (splat (ty_vec128 dst) (f16const _ (u16_from_ieee16 n))))
(const dst (imm64 n)))
(rule (simplify (splat (ty_vec128 dst) (f32const _ (u32_from_ieee32 n))))
(vconst dst (splat32 n)))
(const dst (imm64 n)))
(rule (simplify (splat (ty_vec128 dst) (f64const _ (u64_from_ieee64 n))))
(vconst dst (splat64 n)))
(const dst (imm64 n)))
Comment on lines +228 to +240
Copy link
Member

@fitzgen fitzgen Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably makes sense to leave these vector rules as they were as well. Not clear to me that splatting is the best default for vectors, or if we should always take a 128-bit value and reinterpret it as the vector type, or even if we should have a default constant constructor for vector types. Either way, I think the explicit splatting in the old version was clearer here.


(decl splat8 (u64) Constant)
(rule (splat8 n) (splat16 (u64_or n (u64_shl n 8))))
Expand Down Expand Up @@ -286,13 +288,13 @@

;; Constant fold int-to-float conversions.
(rule (simplify (fcvt_from_uint $F32 (iconst_u _ n)))
(f32const $F32 (f32_from_uint n)))
(const $F32 (imm64_from_ieee32 (f32_from_uint n))))
(rule (simplify (fcvt_from_uint $F64 (iconst_u _ n)))
(f64const $F64 (f64_from_uint n)))
(const $F64 (imm64_from_ieee64 (f64_from_uint n))))
(rule (simplify (fcvt_from_sint $F32 (iconst_s _ n)))
(f32const $F32 (f32_from_sint n)))
(const $F32 (imm64_from_ieee32 (f32_from_sint n))))
(rule (simplify (fcvt_from_sint $F64 (iconst_s _ n)))
(f64const $F64 (f64_from_sint n)))
(const $F64 (imm64_from_ieee64 (f64_from_sint n))))

(decl f32_from_uint (u64) Ieee32)
(extern constructor f32_from_uint f32_from_uint)
Expand All @@ -305,11 +307,11 @@

;; Constant fold bswap of a constant.
(rule (simplify (bswap $I16 (iconst ty (u64_from_imm64 n))))
(subsume (iconst $I16 (imm64 (u64_bswap16 n)))))
(subsume (const $I16 (imm64 (u64_bswap16 n)))))
(rule (simplify (bswap $I32 (iconst ty (u64_from_imm64 n))))
(subsume (iconst $I32 (imm64 (u64_bswap32 n)))))
(subsume (const $I32 (imm64 (u64_bswap32 n)))))
(rule (simplify (bswap $I64 (iconst ty (u64_from_imm64 n))))
(subsume (iconst $I64 (imm64 (u64_bswap64 n)))))
(subsume (const $I64 (imm64 (u64_bswap64 n)))))

(decl pure u64_bswap16 (u64) u64)
(extern constructor u64_bswap16 u64_bswap16)
Expand All @@ -329,117 +331,117 @@
;; TODO: fcmp, fma, demote, promote, to-int ops
(rule (simplify (fadd $F32 (f32const $F32 lhs) (f32const $F32 rhs)))
(if-let r (f32_add lhs rhs))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (fadd $F64 (f64const $F64 lhs) (f64const $F64 rhs)))
(if-let r (f64_add lhs rhs))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (fsub $F32 (f32const $F32 lhs) (f32const $F32 rhs)))
(if-let r (f32_sub lhs rhs))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (fsub $F64 (f64const $F64 lhs) (f64const $F64 rhs)))
(if-let r (f64_sub lhs rhs))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (fmul $F32 (f32const $F32 lhs) (f32const $F32 rhs)))
(if-let r (f32_mul lhs rhs))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (fmul $F64 (f64const $F64 lhs) (f64const $F64 rhs)))
(if-let r (f64_mul lhs rhs))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (fdiv $F32 (f32const $F32 lhs) (f32const $F32 rhs)))
(if-let r (f32_div lhs rhs))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (fdiv $F64 (f64const $F64 lhs) (f64const $F64 rhs)))
(if-let r (f64_div lhs rhs))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))
Comment on lines -332 to +358
Copy link
Member

@fitzgen fitzgen Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And all of the float constants can remain as {f32,f64}const usages because the ergonomics are better and it doesn't make sense to me why we would want a single constant constructor for both integers and floats, given that a constructor has a single type signature for all rules.


(rule (simplify (sqrt $F32 (f32const $F32 n)))
(if-let r (f32_sqrt n))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (sqrt $F64 (f64const $F64 n)))
(if-let r (f64_sqrt n))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (ceil $F32 (f32const $F32 n)))
(if-let r (f32_ceil n))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (ceil $F64 (f64const $F64 n)))
(if-let r (f64_ceil n))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (floor $F32 (f32const $F32 n)))
(if-let r (f32_floor n))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (floor $F64 (f64const $F64 n)))
(if-let r (f64_floor n))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (trunc $F32 (f32const $F32 n)))
(if-let r (f32_trunc n))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (trunc $F64 (f64const $F64 n)))
(if-let r (f64_trunc n))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (nearest $F32 (f32const $F32 n)))
(if-let r (f32_nearest n))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (nearest $F64 (f64const $F64 n)))
(if-let r (f64_nearest n))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))

(rule (simplify (fmin $F16 (f16const $F16 n) (f16const $F16 m)))
(if-let r (f16_min n m))
(subsume (f16const $F32 r)))
(subsume (const $F16 (imm64_from_ieee16 r))))
(rule (simplify (fmin $F32 (f32const $F32 n) (f32const $F32 m)))
(if-let r (f32_min n m))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (fmin $F64 (f64const $F64 n) (f64const $F64 m)))
(if-let r (f64_min n m))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))
(rule (simplify (fmin $F128 (f128const $F128 (ieee128_constant n)) (f128const $F128 (ieee128_constant m))))
(if-let r (f128_min n m))
(subsume (f128const $F128 (ieee128_constant r))))

(rule (simplify (fmax $F16 (f16const $F16 n) (f16const $F16 m)))
(if-let r (f16_max n m))
(subsume (f16const $F16 r)))
(subsume (const $F16 (imm64_from_ieee16 r))))
(rule (simplify (fmax $F32 (f32const $F32 n) (f32const $F32 m)))
(if-let r (f32_max n m))
(subsume (f32const $F32 r)))
(subsume (const $F32 (imm64_from_ieee32 r))))
(rule (simplify (fmax $F64 (f64const $F64 n) (f64const $F64 m)))
(if-let r (f64_max n m))
(subsume (f64const $F64 r)))
(subsume (const $F64 (imm64_from_ieee64 r))))
(rule (simplify (fmax $F128 (f128const $F128 (ieee128_constant n)) (f128const $F128 (ieee128_constant m))))
(if-let r (f128_max n m))
(subsume (f128const $F128 (ieee128_constant r))))

(rule (simplify (fneg $F16 (f16const $F16 n)))
(subsume (f16const $F16 (f16_neg n))))
(subsume (const $F16 (imm64_from_ieee16 (f16_neg n)))))
(rule (simplify (fneg $F32 (f32const $F32 n)))
(subsume (f32const $F32 (f32_neg n))))
(subsume (const $F32 (imm64_from_ieee32 (f32_neg n)))))
(rule (simplify (fneg $F64 (f64const $F64 n)))
(subsume (f64const $F64 (f64_neg n))))
(subsume (const $F64 (imm64_from_ieee64 (f64_neg n)))))
(rule (simplify (fneg $F128 (f128const $F128 (ieee128_constant n))))
(subsume (f128const $F128 (ieee128_constant (f128_neg n)))))

(rule (simplify (fabs $F16 (f16const $F16 n)))
(subsume (f16const $F16 (f16_abs n))))
(subsume (const $F16 (imm64_from_ieee16 (f16_abs n)))))
(rule (simplify (fabs $F32 (f32const $F32 n)))
(subsume (f32const $F32 (f32_abs n))))
(subsume (const $F32 (imm64_from_ieee32 (f32_abs n)))))
(rule (simplify (fabs $F64 (f64const $F64 n)))
(subsume (f64const $F64 (f64_abs n))))
(subsume (const $F64 (imm64_from_ieee64 (f64_abs n)))))
(rule (simplify (fabs $F128 (f128const $F128 (ieee128_constant n))))
(subsume (f128const $F128 (ieee128_constant (f128_abs n)))))

(rule (simplify (fcopysign $F16 (f16const $F16 n) (f16const $F16 m)))
(subsume (f16const $F16 (f16_copysign n m))))
(subsume (const $F16 (imm64_from_ieee16 (f16_copysign n m)))))
(rule (simplify (fcopysign $F32 (f32const $F32 n) (f32const $F32 m)))
(subsume (f32const $F32 (f32_copysign n m))))
(subsume (const $F32 (imm64_from_ieee32 (f32_copysign n m)))))
(rule (simplify (fcopysign $F64 (f64const $F64 n) (f64const $F64 m)))
(subsume (f64const $F64 (f64_copysign n m))))
(subsume (const $F64 (imm64_from_ieee64 (f64_copysign n m)))))
(rule (simplify (fcopysign $F128 (f128const $F128 (ieee128_constant n)) (f128const $F128 (ieee128_constant m))))
(subsume (f128const $F128 (ieee128_constant (f128_copysign n m)))))

Expand Down
16 changes: 16 additions & 0 deletions cranelift/codegen/src/prelude.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,22 @@
(decl u64_from_ieee64 (u64) Ieee64)
(extern extractor infallible u64_from_ieee64 u64_from_ieee64)

;; Construct Ieee* from raw low bits of an Imm64.
(decl ieee16_from_imm64 (Imm64) Ieee16)
(extern constructor ieee16_from_imm64 ieee16_from_imm64)
(decl ieee32_from_imm64 (Imm64) Ieee32)
(extern constructor ieee32_from_imm64 ieee32_from_imm64)
(decl ieee64_from_imm64 (Imm64) Ieee64)
(extern constructor ieee64_from_imm64 ieee64_from_imm64)

;; Construct Imm64 from Ieee* by taking their raw low bits.
(decl imm64_from_ieee16 (Ieee16) Imm64)
(extern constructor imm64_from_ieee16 imm64_from_ieee16)
(decl imm64_from_ieee32 (Ieee32) Imm64)
(extern constructor imm64_from_ieee32 imm64_from_ieee32)
(decl imm64_from_ieee64 (Ieee64) Imm64)
(extern constructor imm64_from_ieee64 imm64_from_ieee64)

;; Match a multi-lane type, extracting (# bits per lane, # lanes) from the given
;; type. Will only match when there is more than one lane.
(decl multi_lane (u32 u32) Type)
Expand Down
Loading