Skip to content

Add sextend_maybe and uextend_maybe to opt ISLE #7710

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

Merged
fitzgen merged 2 commits into from
Dec 20, 2023
Merged

Add sextend_maybe and uextend_maybe to opt ISLE #7710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57634b6
Add a test demonstrating a missed optimization
scottmcm Dec 20, 2023
8ef0b81
Add `[su]extend_maybe` to opt ISLE
scottmcm Dec 20, 2023
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
70 changes: 70 additions & 0 deletions cranelift/codegen/src/opts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::ir::condcodes;
pub use crate::ir::condcodes::{FloatCC, IntCC};
use crate::ir::dfg::ValueDef;
pub use crate::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm8, V128Imm};
use crate::ir::instructions::InstructionFormat;
pub use crate::ir::types::*;
pub use crate::ir::{
dynamic_to_fixed, AtomicRmwOp, BlockCall, Constant, DynamicStackSlot, FuncRef, GlobalValue,
Expand Down Expand Up @@ -121,6 +122,65 @@ where
}
}

#[derive(Default)]
pub(crate) struct MaybeUnaryEtorIter<'a, 'b, 'c> {
opcode: Option<Opcode>,
inner: InstDataEtorIter<'a, 'b, 'c>,
fallback: Option<Value>,
}

impl MaybeUnaryEtorIter<'_, '_, '_> {
fn new(opcode: Opcode, value: Value) -> Self {
debug_assert_eq!(opcode.format(), InstructionFormat::Unary);
Self {
opcode: Some(opcode),
inner: InstDataEtorIter::new(value),
fallback: Some(value),
}
}
}

impl<'a, 'b, 'c> ContextIter for MaybeUnaryEtorIter<'a, 'b, 'c>
where
'b: 'a,
'c: 'b,
{
type Context = IsleContext<'a, 'b, 'c>;
type Output = (Type, Value);

fn next(&mut self, ctx: &mut IsleContext<'a, 'b, 'c>) -> Option<Self::Output> {
debug_assert_ne!(self.opcode, None);
while let Some((ty, inst_def)) = self.inner.next(ctx) {
let InstructionData::Unary { opcode, arg } = inst_def else {
continue;
};
if Some(opcode) == self.opcode {
self.fallback = None;
return Some((ty, arg));
}
}

self.fallback.take().map(|value| {
let ty = generated_code::Context::value_type(ctx, value);
(ty, value)
})
}
}

impl<'a, 'b, 'c> IntoContextIter for MaybeUnaryEtorIter<'a, 'b, 'c>
where
'b: 'a,
'c: 'b,
{
type Context = IsleContext<'a, 'b, 'c>;
type Output = (Type, Value);
type IntoIter = Self;

fn into_context_iter(self) -> Self {
self
}
}

impl<'a, 'b, 'c> generated_code::Context for IsleContext<'a, 'b, 'c> {
isle_common_prelude_methods!();

Expand Down Expand Up @@ -193,4 +253,14 @@ impl<'a, 'b, 'c> generated_code::Context for IsleContext<'a, 'b, 'c> {
let imm = V128Imm(val.to_le_bytes());
self.ctx.func.dfg.constants.insert(imm.into())
}

type sextend_maybe_etor_returns = MaybeUnaryEtorIter<'a, 'b, 'c>;
fn sextend_maybe_etor(&mut self, value: Value, returns: &mut Self::sextend_maybe_etor_returns) {
*returns = MaybeUnaryEtorIter::new(Opcode::Sextend, value);
}

type uextend_maybe_etor_returns = MaybeUnaryEtorIter<'a, 'b, 'c>;
fn uextend_maybe_etor(&mut self, value: Value, returns: &mut Self::uextend_maybe_etor_returns) {
*returns = MaybeUnaryEtorIter::new(Opcode::Uextend, value);
}
}
4 changes: 2 additions & 2 deletions cranelift/codegen/src/opts/icmp.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@

;; Optimize icmp-of-icmp.
(rule (simplify (ne ty
(uextend _ inner @ (icmp ty _ _ _))
(uextend_maybe _ inner @ (icmp ty _ _ _))
(iconst_u _ 0)))
(subsume inner))

(rule (simplify (eq ty
(uextend _ (icmp ty cc x y))
(uextend_maybe _ (icmp ty cc x y))
(iconst_u _ 0)))
(subsume (icmp ty (intcc_complement cc) x y)))

Expand Down
8 changes: 1 addition & 7 deletions cranelift/codegen/src/opts/selects.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
(rule (simplify (select ty cmp@(icmp _ cc x y)
(iconst_u _ 1)
(iconst_u _ 0)))
(uextend_from_i8 ty cmp))
(uextend_maybe ty cmp))
;; if icmp(x, y) { -1 } else { 0 } => uextend(icmp(x, y))
(rule (simplify (select ty cmp@(icmp _ cc x y)
(iconst_s _ -1)
Expand Down Expand Up @@ -75,9 +75,3 @@
(rule (simplify (bor (ty_vec128 ty) (band ty (bnot ty c) y) (band ty x c))) (bitselect ty c x y))
(rule (simplify (bor (ty_vec128 ty) (band ty y (bnot ty c)) (band ty c x))) (bitselect ty c x y))
(rule (simplify (bor (ty_vec128 ty) (band ty y (bnot ty c)) (band ty x c))) (bitselect ty c x y))

;; extend from i8 to i8 is invalid CLIF, so this allows fixing that in the output
;; rather than needing to duplicate rules for the different width categories
(decl uextend_from_i8 (Type Value) Value)
(rule 0 (uextend_from_i8 ty val) (uextend ty val))
(rule 1 (uextend_from_i8 $I8 val) val)
70 changes: 24 additions & 46 deletions cranelift/codegen/src/opts/spaceship.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,145 +12,129 @@
;; x < y ? -1 : x != y ? +1 : 0
(rule (simplify (select ty (ult rty x y)
(iconst_s ty -1)
(ne rty x y)))
(sextend_from_i8 ty (spaceship_u rty x y)))
(rule (simplify (select ty (ult rty x y)
(iconst_s ty -1)
(uextend ty (ne rty x y))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(uextend_maybe ty (ne rty x y))))
(sextend_maybe ty (spaceship_u rty x y)))
;; x < y ? -1 : x <= y ? 0 : +1
;; x < y ? -1 : x > y ? +1 : 0
(rule (simplify (select ty (ult rty x y)
(iconst_s ty -1)
(ugt rty x y)))
(sextend_from_i8 ty (spaceship_u rty x y)))
(rule (simplify (select ty (ult rty x y)
(iconst_s ty -1)
(uextend ty (ugt rty x y))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(uextend_maybe ty (ugt rty x y))))
(sextend_maybe ty (spaceship_u rty x y)))

;; x == y ? 0 : x < y ? -1 : +1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (ult rty x y)
(iconst_s ty -1)
(iconst_s ty 1))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))
;; x == y ? 0 : x <= y ? -1 : +1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (ule rty x y)
(iconst_s ty -1)
(iconst_s ty 1))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))
;; x == y ? 0 : x > y ? +1 : -1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (ugt rty x y)
(iconst_s ty 1)
(iconst_s ty -1))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))
;; x == y ? 0 : x >= y ? +1 : -1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (uge rty x y)
(iconst_s ty 1)
(iconst_s ty -1))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))

;; x > y ? 1 : x < y ? -1 : 0
;; x > y ? 1 : x >= y ? 0 : -1
(rule (simplify (select ty (ugt rty x y)
(iconst_s ty 1)
(ineg rty (ult rty x y))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))
(rule (simplify (select ty (ugt rty x y)
(iconst_s ty 1)
(bmask ty (ult rty x y))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))
;; x > y ? 1 : x != y ? -1 : 0
;; x > y ? 1 : x == y ? 0 : -1
(rule (simplify (select ty (ugt rty x y)
(iconst_s ty 1)
(ineg rty (ne rty x y))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))
(rule (simplify (select ty (ugt rty x y)
(iconst_s ty 1)
(bmask ty (ne rty x y))))
(sextend_from_i8 ty (spaceship_u rty x y)))
(sextend_maybe ty (spaceship_u rty x y)))

;; Same, but for signed comparisons this time

;; x < y ? -1 : x == y ? 0 : +1
;; x < y ? -1 : x != y ? +1 : 0
(rule (simplify (select ty (slt rty x y)
(iconst_s ty -1)
(ne rty x y)))
(spaceship_s rty x y))
(rule (simplify (select ty (slt rty x y)
(iconst_s ty -1)
(uextend ty (ne rty x y))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(uextend_maybe ty (ne rty x y))))
(sextend_maybe ty (spaceship_s rty x y)))
;; x < y ? -1 : x <= y ? 0 : +1
;; x < y ? -1 : x > y ? +1 : 0
(rule (simplify (select ty (slt rty x y)
(iconst_s ty -1)
(sgt rty x y)))
(spaceship_s rty x y))
(rule (simplify (select ty (slt rty x y)
(iconst_s ty -1)
(uextend ty (sgt rty x y))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(uextend_maybe ty (sgt rty x y))))
(sextend_maybe ty (spaceship_s rty x y)))

;; x == y ? 0 : x < y ? -1 : +1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (slt rty x y)
(iconst_s ty -1)
(iconst_s ty 1))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))
;; x == y ? 0 : x <= y ? -1 : +1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (sle rty x y)
(iconst_s ty -1)
(iconst_s ty 1))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))
;; x == y ? 0 : x > y ? +1 : -1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (sgt rty x y)
(iconst_s ty 1)
(iconst_s ty -1))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))
;; x == y ? 0 : x >= y ? +1 : -1
(rule (simplify (select ty (eq rty x y)
(iconst_s ty 0)
(select ty (sge rty x y)
(iconst_s ty 1)
(iconst_s ty -1))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))

;; x > y ? 1 : x < y ? -1 : 0
;; x > y ? 1 : x >= y ? 0 : -1
(rule (simplify (select ty (sgt rty x y)
(iconst_s ty 1)
(ineg rty (slt rty x y))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))
(rule (simplify (select ty (sgt rty x y)
(iconst_s ty 1)
(bmask ty (slt rty x y))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))
;; x > y ? 1 : x != y ? -1 : 0
;; x > y ? 1 : x == y ? 0 : -1
(rule (simplify (select ty (sgt rty x y)
(iconst_s ty 1)
(ineg rty (ne rty x y))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))
(rule (simplify (select ty (sgt rty x y)
(iconst_s ty 1)
(bmask ty (ne rty x y))))
(sextend_from_i8 ty (spaceship_s rty x y)))
(sextend_maybe ty (spaceship_s rty x y)))

;; Then once we have it normalized, we can apply some basic simplifications.
;; For example, a derived `PartialOrd::lt` on a newtype in Rust will essentially
Expand Down Expand Up @@ -208,9 +192,3 @@
(sle ty x y))
(rule (simplify (ne _ (spaceship_u ty x y) (iconst_s _ 1)))
(ule ty x y))

;; extend from i8 to i8 is invalid CLIF, so this allows fixing that in the output
;; rather than needing to duplicate rules for the different width categories
(decl sextend_from_i8 (Type Value) Value)
(rule 0 (sextend_from_i8 ty val) (sextend ty val))
(rule 1 (sextend_from_i8 $I8 val) val)
28 changes: 28 additions & 0 deletions cranelift/codegen/src/prelude_opt.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
;; Prelude definitions specific to the mid-end.

;; Any `extern` definitions here are generally implemented in `src/opts.rs`.

;;;;; eclass and enode access ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Extract any node(s) for the given eclass ID.
Expand Down Expand Up @@ -92,3 +94,29 @@
(if-let $true (u64_le c (ty_umax ty)))
(iconst ty (imm64 c)))
(rule 1 (iconst_u $I128 c) (uextend $I128 (iconst_u $I64 c)))

;; These take `Value`, rather than going through `inst_data_tupled`, because
;; most of the time they want to return the original `Value`, and it would be
;; a waste to need to re-GVN the instruction data in those cases.
(decl multi sextend_maybe_etor (Type Value) Value)
(extern extractor infallible sextend_maybe_etor sextend_maybe_etor)
(decl multi uextend_maybe_etor (Type Value) Value)
(extern extractor infallible uextend_maybe_etor uextend_maybe_etor)

;; Match or Construct a possibly-`uextend`ed value.
;; Gives the extended-to type and inner value when matching something that was
;; extended, or the input value and its type when the value isn't an extension.
;; Useful to write a single pattern that can match things that may or may not
;; have undergone C's "usual arithmetic conversions".
;; When generating values, extending to the same type is invalid CLIF,
;; so this avoids doing that where there's no extension actually needed.
(decl uextend_maybe (Type Value) Value)
(extractor (uextend_maybe ty val) (uextend_maybe_etor ty val))
(rule 0 (uextend_maybe ty val) (uextend ty val))
(rule 1 (uextend_maybe ty val@(value_type ty)) val)

;; Same as `uextend_maybe` above, just for `sextend`.
(decl sextend_maybe (Type Value) Value)
(extractor (sextend_maybe ty val) (sextend_maybe_etor ty val))
(rule 0 (sextend_maybe ty val) (sextend ty val))
(rule 1 (sextend_maybe ty val@(value_type ty)) val)
16 changes: 16 additions & 0 deletions cranelift/filetests/filetests/egraph/icmp.clif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,19 @@ block0(v0: i8):
; v4 = icmp eq v0, v3 ; v3 = 0
; return v4
; }

function %byte_icmp_ucmp(i8) -> i8 {
block0(v0: i8):
v1 = iconst.i8 0
v2 = icmp sge v0, v1
v3 = icmp eq v2, v1
return v3
}

; function %byte_icmp_ucmp(i8) -> i8 fast {
; block0(v0: i8):
; v1 = iconst.i8 0
; v4 = icmp slt v0, v1 ; v1 = 0
; return v4
; }